109.81 KB, image/png
146.20 KB, image/png
1.40 KB, application/x-xpinstall
4.54 KB, patch
|Details | Diff | Splinter Review|
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111207 Firefox/11.0a1 Build ID: 20111207031110 Steps to reproduce: 1/ Update Nightly to the latest version and create a new profile 2/ Install add-on Adblock Plus (adding a blocklist subscription is not necessary) 3/ Go to http://video.webmfiles.org/elephants-dream.webm (WebM video) Actual results: The video starts but the control bar (start/pause, timeline, sound level, fullscreen buttons) is hidden and it's impossible to display the statistics by right clicking on the video. Restart the Nightly in safe mode and video overlays are visible again. Expected results: Add-on Adblock Plus should'nt have to block WebM video overlays.
Created attachment 580000 [details] Minimal content policy for testing This isn't limited to Adblock Plus - the same thing happens with any extension using content policies. I've attached a minimal nsIContentPolicy implementation (does nothing but log nsIContentPolicy calls), the problem is reproducible with this extension as well. I see two exceptions in the Error Console that are apparently related: Error: Permission denied for <http://video.webmfiles.org> to get property Object.dynamicControls Source File: chrome://global/content/bindings/videocontrols.xml Line: 452 Error: this.Utils is undefined Source File: chrome://global/content/bindings/videocontrols.xml Line: 307 Video controls fails to initialize because of these, everything else is just follow-up errors.
Confirming (reproduced in the current Firefox 11.0a1 nightly on Windows 7 x64) and tweaking summary. This is a regression, Firefox 8 works fine.
Wladimir, thanks for clarifying the bug issue. In addition I observed a weird side-effect of this bug when the video is embedded into a webpage like http://www.mozilla.org/projects/firefox/prerelease.html (OGV video, no autoplay). STR: 1/ Start the latest Nightly in safe mode with new profile and add-on Adblock Plus. 2/ Open 2 tabs: ++ 1st tab: open http://www.mozilla.org/projects/firefox/prerelease.html (embedded video, no autoplay) ++ 2nd tab: open http://videos-cdn.mozilla.net/serv/qa/qa-360p-mp4.theora.ogv (direct video link, autoplay) In both tabs, video controls are visible. 3/ Close Nightly and restart it normally (NO safe mode). 4/ Even if Adblock Plus is enabled, the 1st tab DISPLAYS video controls on embedded video and the 2nd tab doesn't display video controls on direct video (according to this bug). 5/ Restart Nightly normally a 2nd time, in both tabs, video controls are not displayed (according to this bug).
Regression range with STR of comment #0 and Minimal content policy of comment #3 . Regression window(m-c) Works: http://hg.mozilla.org/mozilla-central/rev/fd478c02c29c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111115 Firefox/11.0a1 ID:20111115110849 Fails: http://hg.mozilla.org/mozilla-central/rev/3ea216303184 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111115 Firefox/11.0a1 ID:20111115135750 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fd478c02c29c&tochange=3ea216303184 Regression window(fx-team) Works: http://hg.mozilla.org/integration/fx-team/rev/7e1f046b173b Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111110 Firefox/11.0a1 ID:20111110040711 Fails: http://hg.mozilla.org/integration/fx-team/rev/15c2cf2f47f4 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111111 Firefox/11.0a1 ID:20111111040857 Pushlog: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=7e1f046b173b&tochange=15c2cf2f47f4 Triggered by; Bug 700856 and Bug 700854
(In reply to Alice0775 White from comment #7) > P.S.; I think the problem step 5 of comment #5 is different bug, and maybe > Bug 701662. Thanks for pointing it, that's exactly I'm meeting in your comment https://bugzilla.mozilla.org/show_bug.cgi?id=701662#c19
Also if you right click and use the controls of the menu the controls don't work.
Requesting "tracking-firefox11", as this is a regression in Firefox 11 (which is migrating from Nightly to Aurora in a few days).
Bug 711675 is probably related.
Hmm. Are we now ending up with content policy checks (for the external stylesheet) before we've set up the script global for the document? That's what's caused this problem in the past.... Jared, are you owning this?
(In reply to Boris Zbarsky (:bz) from comment #14) > Hmm. Are we now ending up with content policy checks (for the external > stylesheet) before we've set up the script global for the document? That's > what's caused this problem in the past.... > Jared, are you owning this? I think this is over my head. Would the changes necessary be in videocontrols.xml or lower level?
It would need to be fixed in the code that starts the loads, so in this case in VideoDocument::CreateSyntheticVideoDocument Basically, that code runs _very_ early, before the window for the document has been created. Doing anything in there that might execute script (including starting loads of any sort) is unsafe. So in particular, the patch for bug 700854 is wrong. Maybe we could change the timing of that call to also happen during SetScriptGlobalObject just like the document-element-inserted notification. Someone who understands this code would need to check that. The other option is to just postpone the stylesheet load until after we have a script global.
s/videocontrols.xml/VideoDocument.cpp in comment #16 Thanks for the pointers Boris. I can try to take this bug.
Might bug 693206 help to avoid this? Easy enough to try, I suppose.
(In reply to Justin Dolske [:Dolske] from comment #19) > Might bug 693206 help to avoid this? Easy enough to try, I suppose. I've tried simply replacing the <field> with a <property>, but I didn't notice any change.
Created attachment 583947 [details] [diff] [review] WIP of patch for bug 708431 I have installed the minimal content policy add-on that was attached to this bug. Before this patch, the browser would hang when visiting a webm file directly (in debug mode). After this patch, the video now loads but the controls still fail to appear. When I hover over the video, this warning message prints to the console. It seems to print to the console for every mouse movement. > WARNING: NS_ENSURE_TRUE(piTarget) failed: file c:/mozilla-central/obj-dir/content/events/src/../../../../content/events/src/nsEventDispatcher.cpp, line 531 I have also made the same change for ImageDocuments, under the assumption that if it was bad to do this for VideoDocuments, then we should probably fix it in ImageDocuments also.
Comment on attachment 583947 [details] [diff] [review] WIP of patch for bug 708431 Clearing review, I forgot to call SetScriptGlobalObject on the MediaDocument.
Created attachment 583953 [details] [diff] [review] Patch for bug 708431
Comment on attachment 583953 [details] [diff] [review] Patch for bug 708431 I added the call to MediaDocument::SetScriptGlobalObject and the warning is now gone. I've tested this with the "Minimal content policy for testing" attachment and the controls with and without the addon enabled.
Comment on attachment 583953 [details] [diff] [review] Patch for bug 708431 You should probably only do the LinkStylesheet thing when setting the script global, not when unsetting it.
Created attachment 584000 [details] [diff] [review] Patch for bug 708431 v1.1 Only linking the stylesheet when the script global is being set.
(In reply to Jared Wein [:jwein and :jaws] from comment #26) > Created attachment 584000 [details] [diff] [review] > Patch for bug 708431 v1.1 bz: review ping?
Comment on attachment 584000 [details] [diff] [review] Patch for bug 708431 v1.1 r=me. Silly holidays!
Comment on attachment 584000 [details] [diff] [review] Patch for bug 708431 v1.1 [Approval Request Comment] Regression caused by (bug #): bug 700854 User impact if declined: Video controls won't work, and a small memory leak is introduced (bug 711675). Testing completed (on m-c, etc.): This has been on m-c since 1/8. Risk to taking this patch (and alternatives if risky): No risks that I know of.
Comment on attachment 584000 [details] [diff] [review] Patch for bug 708431 v1.1 [Triage Comment] Low risk - approving for aurora.
Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0 Verified the fix on the above builds: Adblock Plus, NoScript(select "Allow all on the page") and Greasemonkey don't block the display of video controls. Marking as VERIFIED
(In reply to Mihaela Velimiroviciu [QA] from comment #34) > Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0 > > Verified the fix on the above builds: Adblock Plus, NoScript(select "Allow > all on the page") and Greasemonkey don't block the display of video > controls. > Marking as VERIFIED Forgot the Mac and Ubuntu builds: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0 Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0