Multimedia player: Fix AJAX race condition in Firefox #9329
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EricDunsworth
force-pushed
the
multimedia-ajax-init-race-firefox
branch
3 times, most recently
from
3c574b1
to
03c3274
Compare
Players brought in via the Data Ajax plugin sometimes tried initializing themselves twice in Firefox. When it happened, the first attempt occurred before the AJAXed-in player had received a "wb-auto-*" ID - resulting in the player and media element's IDs getting set to "undefined" and "undefined-md" respectively. The second attempt then treated the player's transformed HTML markup as if it was the initial markup. That caused several issues, such as a "video" class getting added to audio players (producing a large play icon overlapping the player controls), ID mismatches and a JavaScript console error related to the media variable being undefined. This resolves it via the following changes: * Add a check to ensure the media variable is truthy before proceeding with initialization * Move most variable declarations after the media check * Get an ID from wb.getId() if the player doesn't already have one * Note: Getting an ID at this point will prevent the second initialization from occurring. But the media check is still necessary for players that use hardcoded IDs. * Don't check media.currentSrc in the condition that triggers renderUIEvent (speeds things up by triggering it on the first init instead of the second, higher-level media check makes this one redundant)
EricDunsworth
force-pushed
the
multimedia-ajax-init-race-firefox
branch
from
03c3274
to
7979641
Compare
|
Just got around to doing a YouTube test... seems to initialize AJAXed-in YT players twice (resulting in two sets of player controls). Will need to resolve that before promoting this PR out of draft. |
|
#9616 caught my eye. It might have an impact on the underlying issue this PR was trying to fix. Just noting it here for whenever I get around to refocusing my attention on this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Players brought in via the Data Ajax plugin sometimes tried initializing themselves twice in Firefox.
When it happened, the first attempt occurred before the AJAXed-in player had received a "wb-auto-*" ID - resulting in the player and media element's IDs getting set to "undefined" and "undefined-md" respectively.
The second attempt then treated the player's transformed HTML markup as if it was the initial markup. That caused several issues, such as a "video" class getting added to audio players (producing a large play icon overlapping the player controls), ID mismatches and a JavaScript console error related to the
mediavariable being undefined.This resolves it via the following changes:
mediavariable is truthy before proceeding with initializationmediacheckwb.getId()if the player doesn't already have onemediacheck is still necessary for players that use hardcoded IDs.media.currentSrcin the condition that triggersrenderUIEvent(speeds things up by triggering it on the first init instead of the second, higher-levelmediacheck makes this one redundant)