RESOLVED FIXED 79746
Main rendering steps for displaying the cue boxes
https://bugs.webkit.org/show_bug.cgi?id=79746
Summary Main rendering steps for displaying the cue boxes
Victor Carbune
Reported 2012-02-27 22:58:56 PST
Bug for implementing the "outer" steps 1 - 11 of the WebVTT cue text rendering rules. "The rules are as follows: 1. If the media element is an audio element, or is another playback mechanism with no rendering area, abort these steps. There is nothing to render. [...] 11. Return output."
Attachments
Preliminary patch (29.03 KB, patch)
2012-03-01 16:36 PST, Victor Carbune
no flags
Updated patch (33.10 KB, patch)
2012-03-07 14:59 PST, Victor Carbune
no flags
Patch (39.98 KB, patch)
2012-03-09 14:21 PST, Victor Carbune
no flags
Small fixed and corrected ChangeLog (39.08 KB, patch)
2012-03-11 16:10 PDT, Victor Carbune
no flags
Victor Carbune
Comment 1 2012-03-01 16:36:42 PST
Created attachment 129769 [details] Preliminary patch
Eric Carlson
Comment 2 2012-03-02 10:59:40 PST
Comment on attachment 129769 [details] Preliminary patch View in context: https://bugs.webkit.org/attachment.cgi?id=129769&action=review > Source/WebCore/ChangeLog:12 > + * css/mediaControls.css: > + (video::-webkit-media-text-track-display): Adjusted text color. Please say why the color was changed. > Source/WebCore/html/shadow/MediaControlElements.cpp:1203 > + if (!toParentMediaElement(this)->isVideo()) > + return; > + > + // 2. Let video be the media element or other playback mechanism. > + HTMLVideoElement* video = static_cast<HTMLVideoElement*>(toParentMediaElement(this)); Nit: you can avoid one of the calls to toParentMediaElement() by changing this slightly. > Source/WebCore/html/shadow/MediaControlElements.cpp:1214 > + // FIXME (should be copying the media controls box into a transparent box): > + if (video->controls()) > + output.append(video->mediaControls()); Could you achieve the desired effect, keeping the captions above the default controls, by putting the controls and captions in the same box with appropriate alignment (controls always at the bottom, etc)? This way, we could animate the containing box when the controls visibility is changed and the captions and controls will move in sync. > Source/WebCore/html/shadow/MediaControlElements.cpp:1259 > + if (cue->getDisplayState().size()) { > + ExceptionCode ec; > + appendChild(cue->getDisplayState()[0], ec, true); > + Calling getDisplayState() twice is unnecessary, you might as well put it into a local. Why is only the first element in the vector used? > Source/WebCore/html/track/TextTrackCue.cpp:409 > +Vector<RefPtr<HTMLDivElement> > TextTrackCue::getDisplayState() I don't think this is a great name for this method, I would never guess that "getDisplayState" returns the display element(s). Maybe "getDisplayNodes()", or "getDisplayTree()", or ... I don't understand why this returns a Vector of elements. Couldn't it return a single root node with however many child nodes it needs to display the cue? > Source/WebCore/html/track/TextTrackCue.cpp:412 > + if (m_displayBoxes.size() || !m_scriptExecutionContext->isDocument()) > + return m_displayBoxes; Don't you need to clear m_displayBoxes in TextTrackCue::cueDidChange() so we update when a cue is modified? > Source/WebCore/html/track/TextTrackCue.cpp:425 > + // The document tree is the tree of WebVTT Node Objects rooted at nodes. > + simpleBox->appendChild(m_documentFragment, ASSERT_NO_EXCEPTION, true); Won't this allow a arbitrary markup in a cue? What does tc028-unsupported-markup.vtt look like with this? > LayoutTests/media/track/track-cue-rendering-expected.txt:33 > -EXPECTED (getComputedStyle(textTrackDisplayElement(video, 'container')).fontSize == '12px') OK > +EXPECTED (getComputedStyle(mediaControlsElement(internals.shadowRoot(video).firstChild, '-webkit-media-text-track-container')).fontSize == '12px') OK I would rather have specifics about the shadow hierarchy in a helper function so less will have to change if (when) we change it. Can't you use textTrackDisplayElement() by changing it to work when the "id" parameter is missing and call "textTrackDisplayElement(video)" here?
Victor Carbune
Comment 3 2012-03-07 14:59:55 PST
Created attachment 130702 [details] Updated patch
Victor Carbune
Comment 4 2012-03-07 15:16:12 PST
(In reply to comment #2) > (From update of attachment 129769 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=129769&action=review > > > Source/WebCore/ChangeLog:12 > > + * css/mediaControls.css: > > + (video::-webkit-media-text-track-display): Adjusted text color. > > Please say why the color was changed. Done. > > Source/WebCore/html/shadow/MediaControlElements.cpp:1203 > > + if (!toParentMediaElement(this)->isVideo()) > > + return; > > + > > + // 2. Let video be the media element or other playback mechanism. > > + HTMLVideoElement* video = static_cast<HTMLVideoElement*>(toParentMediaElement(this)); > > Nit: you can avoid one of the calls to toParentMediaElement() by changing this slightly. Done. > > Source/WebCore/html/shadow/MediaControlElements.cpp:1214 > > + // FIXME (should be copying the media controls box into a transparent box): > > + if (video->controls()) > > + output.append(video->mediaControls()); > > Could you achieve the desired effect, keeping the captions above the default controls, by putting the controls and captions in the same box with appropriate alignment (controls always at the bottom, etc)? This way, we could animate the containing box when the controls visibility is changed and the captions and controls will move in sync. Indeed, I think it's a much better approach. The media-controls now has display:-webkit-box such that the two boxes (track container and controls panel) are automatically positioned. The only drawback of using -webkit-box is that the captions don't transition to the bottom (but I think it's cleaner to use -webkit-box, rather than manually extending the size of the container and so on). > > Source/WebCore/html/shadow/MediaControlElements.cpp:1259 > > + if (cue->getDisplayState().size()) { > > + ExceptionCode ec; > > + appendChild(cue->getDisplayState()[0], ec, true); > > + > > Calling getDisplayState() twice is unnecessary, you might as well put it into a local. > > Why is only the first element in the vector used? > > > Source/WebCore/html/track/TextTrackCue.cpp:409 > > +Vector<RefPtr<HTMLDivElement> > TextTrackCue::getDisplayState() > > I don't think this is a great name for this method, I would never guess that "getDisplayState" returns the display element(s). Maybe "getDisplayNodes()", or "getDisplayTree()", or ... > > I don't understand why this returns a Vector of elements. Couldn't it return a single root node with however many child nodes it needs to display the cue? I read the spec again more closely - I initially thought that re-positioning is done for each individual box (children of the root node), but it seems the whole box itself is moved. I have changed the name to getDisplayTree and returns a single root node. > > Source/WebCore/html/track/TextTrackCue.cpp:412 > > + if (m_displayBoxes.size() || !m_scriptExecutionContext->isDocument()) > > + return m_displayBoxes; > > Don't you need to clear m_displayBoxes in TextTrackCue::cueDidChange() so we update when a cue is modified? Marked a boolean variable false. > > Source/WebCore/html/track/TextTrackCue.cpp:425 > > + // The document tree is the tree of WebVTT Node Objects rooted at nodes. > > + simpleBox->appendChild(m_documentFragment, ASSERT_NO_EXCEPTION, true); > > Won't this allow a arbitrary markup in a cue? What does tc028-unsupported-markup.vtt look like with this? No, this doesn't happen - the WebVTT spec 3.4 is what gets in the fragment (so anything not appropriate is stripped out): http://dev.w3.org/html5/webvtt/#webvtt-cue-text-dom-construction-rules This is implemented in WebVTTParser::createDocumentFragmentFromCueText. I have also changed to not use m_documentFragment, as this is what is returned to the developer / user when it calls ::getCueAsHTML(), so I have used a different instance here. > > LayoutTests/media/track/track-cue-rendering-expected.txt:33 > > -EXPECTED (getComputedStyle(textTrackDisplayElement(video, 'container')).fontSize == '12px') OK > > +EXPECTED (getComputedStyle(mediaControlsElement(internals.shadowRoot(video).firstChild, '-webkit-media-text-track-container')).fontSize == '12px') OK > > I would rather have specifics about the shadow hierarchy in a helper function so less will have to change if (when) we change it. Can't you use textTrackDisplayElement() by changing it to work when the "id" parameter is missing and call "textTrackDisplayElement(video)" here? Done.
WebKit Review Bot
Comment 5 2012-03-07 17:28:36 PST
Comment on attachment 130702 [details] Updated patch Attachment 130702 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11862025 New failing tests: media/media-document-audio-repaint.html media/video-no-audio.html media/controls-strict.html media/video-volume-slider.html media/controls-styling.html media/video-display-toggle.html media/audio-controls-rendering.html media/video-zoom-controls.html media/video-controls-rendering.html media/controls-without-preload.html media/media-controls-clone.html fast/layers/video-layer.html media/video-empty-source.html media/video-playing-and-pause.html media/controls-after-reload.html
Victor Carbune
Comment 6 2012-03-08 07:16:48 PST
(In reply to comment #5) > (From update of attachment 130702 [details]) > Attachment 130702 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/11862025 > > New failing tests: > media/media-document-audio-repaint.html > media/video-no-audio.html > media/controls-strict.html > media/video-volume-slider.html > media/controls-styling.html > media/video-display-toggle.html > media/audio-controls-rendering.html > media/video-zoom-controls.html > media/video-controls-rendering.html > media/controls-without-preload.html > media/media-controls-clone.html > fast/layers/video-layer.html > media/video-empty-source.html > media/video-playing-and-pause.html > media/controls-after-reload.html These come from replacing the display:block with -webkit-box; From the image diffs it seems the functionality itself is preserved, but there are some unnoticeable pixels different.
Eric Carlson
Comment 7 2012-03-08 10:42:03 PST
Comment on attachment 130702 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=130702&action=review I made a few minor suggestions, but this looks great overall! It is probably worth regenerating the ChangeLogs to make sure all changes are documented. > Source/WebCore/ChangeLog:13 > + * css/mediaControls.css: > + (video::-webkit-media-text-track-display): Adjusted text color to match > + the color required in the WebVTT spec (section 3.5.1 'color' property) It is worth mentioning the changes you made to position the controls. > Source/WebCore/css/mediaControls.css:44 > - display: block; > + display: -webkit-box; > direction: ltr; > + -webkit-box-align: start; > + -webkit-box-pack: end; > + -webkit-box-orient: vertical; I expect this is the source of the chromium test failures? > Source/WebCore/css/mediaControlsChromium.css:45 > - position: absolute; > + position: relative; > overflow: visible; This didn't make it into the ChangeLog. > Source/WebCore/html/shadow/MediaControlElements.cpp:43 > #include "HTMLMediaElement.h" > +#include "HTMLVideoElement.h" > #include "HTMLNames.h" Includes should be sorted. > Source/WebCore/html/shadow/MediaControlElements.cpp:184 > + stopTimer(); > + > + double duration = document()->page() ? document()->page()->theme()->mediaControlsFadeOutDuration() : 0; > + m_transitionTimer.startOneShot(duration); It would be good to have a brief comment here about why we need to use a timer for this. As you know, this confused at least me ;-) The timer functions also didn't make it into the ChangeLog. > Source/WebCore/html/shadow/MediaControlElements.cpp:1244 > + // 4. If the user agent is exposing a user interface for video, add to > + // output one or more completely transparent positioned CSS block boxes that > + // cover the same region as the user interface. > + > + // 5. If the last time these rules were run, the user agent was not exposing > + // a user interface for video, but now it is, let reset be true. Otherwise, > + // let reset be false. A brief comment about why you don't have to do anything explicit for these steps would be helpful. > Source/WebCore/html/shadow/MediaControlElements.cpp:1256 > + // Redraw > + removeChildren(); This comment a bit terse. > Source/WebCore/html/shadow/MediaControlElements.cpp:1260 > + // 9. If reset is false, then, for each text track cue cue in cues: if cue's > + // text track cue display state has a set of CSS boxes, then add those boxes > + // to output, and remove cue from cues. A brief comment about why you don't have to do anything explicit for these steps would be helpful. > Source/WebCore/html/track/TextTrackCue.cpp:426 > + // 10.11. Apply the terms of the CSS specifications to nodes within the > + // following constraints, thus obtaining a set of CSS boxes positioned > + // relative to an initial containing block: > + RefPtr<HTMLDivElement> m_displayTree = HTMLDivElement::create(document); Could you empty the div if it already exists instead of deleting and allocating it every time a cue changes? > Source/WebCore/html/track/TextTrackCue.cpp:429 > + // The document tree is the tree of WebVTT Node Objects rooted at nodes. > + RefPtr<DocumentFragment> documentTree = WebVTTParser::create(0, m_scriptExecutionContext)->createDocumentFragmentFromCueText(m_content); Why not use TextTrackCue::getCueAsHTML() instead? That way we will use the fragment if it has already been created (always the case for a cue that has not been modified by script). > LayoutTests/media/media-controls.js:35 > -function textTrackDisplayElement(parentElement, id) > +function textTrackDisplayElement(parentElement) Nit: I think this would be easier to understand if it kept the "id" parameter.
Victor Carbune
Comment 8 2012-03-09 14:21:15 PST
Created attachment 131107 [details] Patch More refactoring and marked tests for rebaselining
Victor Carbune
Comment 9 2012-03-09 14:28:15 PST
(In reply to comment #7) > (From update of attachment 130702 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=130702&action=review > > I made a few minor suggestions, but this looks great overall! It is probably worth regenerating the ChangeLogs to make sure all changes are documented. > > > Source/WebCore/ChangeLog:13 > > + * css/mediaControls.css: > > + (video::-webkit-media-text-track-display): Adjusted text color to match > > + the color required in the WebVTT spec (section 3.5.1 'color' property) > > It is worth mentioning the changes you made to position the controls. Done. > > Source/WebCore/css/mediaControls.css:44 > > - display: block; > > + display: -webkit-box; > > direction: ltr; > > + -webkit-box-align: start; > > + -webkit-box-pack: end; > > + -webkit-box-orient: vertical; > > I expect this is the source of the chromium test failures? Yes, just these lines and functional visual changes. > > Source/WebCore/css/mediaControlsChromium.css:45 > > - position: absolute; > > + position: relative; > > overflow: visible; > > This didn't make it into the ChangeLog. Regenerated changelogs. > > Source/WebCore/html/shadow/MediaControlElements.cpp:43 > > #include "HTMLMediaElement.h" > > +#include "HTMLVideoElement.h" > > #include "HTMLNames.h" > > Includes should be sorted. Done. > > Source/WebCore/html/shadow/MediaControlElements.cpp:184 > > + stopTimer(); > > + > > + double duration = document()->page() ? document()->page()->theme()->mediaControlsFadeOutDuration() : 0; > > + m_transitionTimer.startOneShot(duration); > > It would be good to have a brief comment here about why we need to use a timer for this. As you know, this confused at least me ;-) Added. > The timer functions also didn't make it into the ChangeLog. Regenerated ChangeLogs. > > Source/WebCore/html/shadow/MediaControlElements.cpp:1244 > > + // 4. If the user agent is exposing a user interface for video, add to > > + // output one or more completely transparent positioned CSS block boxes that > > + // cover the same region as the user interface. > > + > > + // 5. If the last time these rules were run, the user agent was not exposing > > + // a user interface for video, but now it is, let reset be true. Otherwise, > > + // let reset be false. > > A brief comment about why you don't have to do anything explicit for these steps would be helpful. Done. > > Source/WebCore/html/shadow/MediaControlElements.cpp:1256 > > + // Redraw > > + removeChildren(); > > This comment a bit terse. Changed. > > Source/WebCore/html/shadow/MediaControlElements.cpp:1260 > > + // 9. If reset is false, then, for each text track cue cue in cues: if cue's > > + // text track cue display state has a set of CSS boxes, then add those boxes > > + // to output, and remove cue from cues. > > A brief comment about why you don't have to do anything explicit for these steps would be helpful. > > > Source/WebCore/html/track/TextTrackCue.cpp:426 > > + // 10.11. Apply the terms of the CSS specifications to nodes within the > > + // following constraints, thus obtaining a set of CSS boxes positioned > > + // relative to an initial containing block: > > + RefPtr<HTMLDivElement> m_displayTree = HTMLDivElement::create(document); > > Could you empty the div if it already exists instead of deleting and allocating it every time a cue changes? Yes, it indeed makes a lot of sense not adding and deleting nodes all the time. However, we certainly don't want to end up with a lot of empty divs (for each cue in the track), so they should be removed even if they're empty. I have changed the methods to keep the div appended only while the cue has the active flag set, and called remove() on the displayTree when the flag is unset (within TextTrackCue::setIsActive). > > Source/WebCore/html/track/TextTrackCue.cpp:429 > > + // The document tree is the tree of WebVTT Node Objects rooted at nodes. > > + RefPtr<DocumentFragment> documentTree = WebVTTParser::create(0, m_scriptExecutionContext)->createDocumentFragmentFromCueText(m_content); > > Why not use TextTrackCue::getCueAsHTML() instead? That way we will use the fragment if it has already been created (always the case for a cue that has not been modified by script). Done. > > LayoutTests/media/media-controls.js:35 > > -function textTrackDisplayElement(parentElement, id) > > +function textTrackDisplayElement(parentElement) > > Nit: I think this would be easier to understand if it kept the "id" parameter. Done.
Eric Carlson
Comment 10 2012-03-10 13:38:58 PST
Comment on attachment 131107 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131107&action=review This is a *terrific* patch, thank you! I am marking this cq- only because of the duplicate entry in the layout test ChangeLog. > Source/WebCore/html/shadow/MediaControlElements.cpp:1224 > + LOG(Media, "MediaControlTextTrackContainerElement::updateDisplay()"); Nit: This function is called frequently enough that this logging generates a *lot* of output. We should remove it in a future check in. > Source/WebCore/html/shadow/MediaControlElements.cpp:1285 > + if (!contains(displayTree.get()) && displayTree->hasChildNodes()) > + appendChild(displayTree, ASSERT_NO_EXCEPTION, true); Nit: I think the "hasChildNodes()" test will be faster than "contains()", so it should be the first test in the expression. This would be good to fix the next time this function is modified. > Source/WebCore/html/track/TextTrackCue.cpp:424 > + if (!m_displayTreeShouldChange || !m_scriptExecutionContext->isDocument()) Nit: unless a scriptExecutionContext can change type, we should "ASSERT(m_scriptExecutionContext->isDocument())" in the constructor instead of checking it every time through here. Again, this can be changed the next time this file is modified. > Source/WebCore/html/track/TextTrackCue.cpp:441 > + m_displayTree->appendChild(getCueAsHTML(), ASSERT_NO_EXCEPTION, true); Why not use m_documentFragment directly, getCueAsHTML creates a new fragment every time? If it isn't possible to use m_documentFragment, why do we cache it in getCueAsHTML at all? Either way, this is an optimization and can be done in a follow up patch. > LayoutTests/ChangeLog:3175 > +2012-03-08 Victor Carbune <vcarbune@adobe.com> > + > + Updated LayoutTests for basic rendering of cues. > + https://bugs.webkit.org/show_bug.cgi?id=79746 > + > + Reviewed by NOBODY (OOPS!). > + > + * media/media-controls.js: > + (mediaControlsElement): > + (textTrackDisplayElement): > + * media/track/track-cue-mutable-text-expected.txt: > + * media/track/track-cue-nothing-to-render-expected.txt: > + * media/track/track-cue-rendering-expected.txt: > + * media/track/track-cue-rendering.html: > + Oops, the ChangeLog should not have two entries.
Victor Carbune
Comment 11 2012-03-11 16:10:30 PDT
Created attachment 131260 [details] Small fixed and corrected ChangeLog
Victor Carbune
Comment 12 2012-03-11 16:17:20 PDT
(In reply to comment #10) > (From update of attachment 131107 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=131107&action=review > > This is a *terrific* patch, thank you! I am marking this cq- only because of the duplicate entry in the layout test ChangeLog. > > > Source/WebCore/html/shadow/MediaControlElements.cpp:1224 > > + LOG(Media, "MediaControlTextTrackContainerElement::updateDisplay()"); > > Nit: This function is called frequently enough that this logging generates a *lot* of output. We should remove it in a future check in. Fixed. > > Source/WebCore/html/shadow/MediaControlElements.cpp:1285 > > + if (!contains(displayTree.get()) && displayTree->hasChildNodes()) > > + appendChild(displayTree, ASSERT_NO_EXCEPTION, true); > > Nit: I think the "hasChildNodes()" test will be faster than "contains()", so it should be the first test in the expression. This would be good to fix the next time this function is modified. Fixed. > > Source/WebCore/html/track/TextTrackCue.cpp:424 > > + if (!m_displayTreeShouldChange || !m_scriptExecutionContext->isDocument()) > > Nit: unless a scriptExecutionContext can change type, we should "ASSERT(m_scriptExecutionContext->isDocument())" in the constructor instead of checking it every time through here. Again, this can be changed the next time this file is modified. Fixed. > > Source/WebCore/html/track/TextTrackCue.cpp:441 > > + m_displayTree->appendChild(getCueAsHTML(), ASSERT_NO_EXCEPTION, true); > > Why not use m_documentFragment directly, getCueAsHTML creates a new fragment every time? If it isn't possible to use m_documentFragment, why do we cache it in getCueAsHTML at all? > > Either way, this is an optimization and can be done in a follow up patch. Here's my approach: * m_documentFragment is cached in getCueAsHTML() because it's faster to clone it, rather than re-parse & re-create it (re-parsing is required only when text contents have changed). * m_displayTree is the cached 'internal version' of m_documentFragment for the shadow dom (it gets reprocessed only when m_displayTreeShouldChange is true), which later will have additional styling or other nodes we need for the cue rendering rules. This implies that m_documentFragment can't be used directly in m_displayTree because calling getCueAsHTML() needs to have a clean version of it. > > LayoutTests/ChangeLog:3175 > > +2012-03-08 Victor Carbune <vcarbune@adobe.com> > > + > > + Updated LayoutTests for basic rendering of cues. > > + https://bugs.webkit.org/show_bug.cgi?id=79746 > > + > > + Reviewed by NOBODY (OOPS!). > > + > > + * media/media-controls.js: > > + (mediaControlsElement): > > + (textTrackDisplayElement): > > + * media/track/track-cue-mutable-text-expected.txt: > > + * media/track/track-cue-nothing-to-render-expected.txt: > > + * media/track/track-cue-rendering-expected.txt: > > + * media/track/track-cue-rendering.html: > > + > > Oops, the ChangeLog should not have two entries. Sorry about this, it seems I'm constantly having issues merging ChangeLogs ;)
Eric Carlson
Comment 13 2012-03-11 20:20:29 PDT
(In reply to comment #12) > (In reply to comment #10) > > > > Why not use m_documentFragment directly, getCueAsHTML creates a new fragment every time? If it isn't possible to use m_documentFragment, why do we cache it in getCueAsHTML at all? > > > > Either way, this is an optimization and can be done in a follow up patch. > Here's my approach: > * m_documentFragment is cached in getCueAsHTML() because it's faster to clone it, rather than re-parse & re-create it (re-parsing is required only when text contents have changed). > * m_displayTree is the cached 'internal version' of m_documentFragment for the shadow dom (it gets reprocessed only when m_displayTreeShouldChange is true), which later will have additional styling or other nodes we need for the cue rendering rules. > > This implies that m_documentFragment can't be used directly in m_displayTree because calling getCueAsHTML() needs to have a clean version of it. > That makes sense. Lets add a comment explaining this the next time this file is modified.
WebKit Review Bot
Comment 14 2012-03-11 21:47:46 PDT
Comment on attachment 131260 [details] Small fixed and corrected ChangeLog Clearing flags on attachment: 131260 Committed r110409: <http://trac.webkit.org/changeset/110409>
WebKit Review Bot
Comment 15 2012-03-11 21:47:52 PDT
All reviewed patches have been landed. Closing bug.
Jer Noble
Comment 16 2012-04-13 14:45:23 PDT
This patch broke the ability of the media controls to be dragged in full screen mode. This is due to going from absolute to relative positioning in mediaControls.css.
Eric Carlson
Comment 17 2012-04-16 10:29:37 PDT
(In reply to comment #16) > This patch broke the ability of the media controls to be dragged in full screen mode. This is due to going from absolute to relative positioning in mediaControls.css. Jer fixed the issue with https://bugs.webkit.org/show_bug.cgi?id=81176.
Victor Carbune
Comment 18 2012-04-16 14:20:42 PDT
(In reply to comment #17) > (In reply to comment #16) > > This patch broke the ability of the media controls to be dragged in full screen mode. This is due to going from absolute to relative positioning in mediaControls.css. > > Jer fixed the issue with https://bugs.webkit.org/show_bug.cgi?id=81176. Thank you, Jer.
Note You need to log in before you can comment on or make changes to this bug.