Last Comment Bug 708431 - Video controls fail to initialize if an extension implementing nsIContentPolicy is present (e.g. Adblock Plus, NoScript, GreaseMonkey)
: Video controls fail to initialize if an extension implementing nsIContentPoli...
Status: VERIFIED FIXED
[qa!] [testday-20120203]
: regression, verified-beta
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla12
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
Mentors:
http://video.webmfiles.org/elephants-...
: 711499 712264 712673 (view as bug list)
Depends on:
Blocks: abp 700854 700856 702800 711675 713555
  Show dependency treegraph
 
Reported: 2011-12-07 15:07 PST by Loic
Modified: 2012-02-03 07:25 PST (History)
24 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected
unaffected
+
verified


Attachments
WebM video with Adblock Plus enabled (109.81 KB, image/png)
2011-12-07 15:08 PST, Loic
no flags Details
WebM video with Adblock Plus disabled (146.20 KB, image/png)
2011-12-07 15:09 PST, Loic
no flags Details
Minimal content policy for testing (1.40 KB, application/x-xpinstall)
2011-12-08 03:56 PST, Wladimir Palant
no flags Details
WIP of patch for bug 708431 (4.35 KB, patch)
2011-12-22 15:03 PST, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch for bug 708431 (4.53 KB, patch)
2011-12-22 15:28 PST, Jared Wein [:jaws] (please needinfo? me)
bzbarsky: review-
Details | Diff | Splinter Review
Patch for bug 708431 v1.1 (4.54 KB, patch)
2011-12-22 19:51 PST, Jared Wein [:jaws] (please needinfo? me)
bzbarsky: review+
roc: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Loic 2011-12-07 15:07:40 PST
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.
Comment 1 Loic 2011-12-07 15:08:45 PST
Created attachment 579858 [details]
WebM video with Adblock Plus enabled
Comment 2 Loic 2011-12-07 15:09:30 PST
Created attachment 579859 [details]
WebM video with Adblock Plus disabled
Comment 3 Wladimir Palant 2011-12-08 03:56:37 PST
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.
Comment 4 Wladimir Palant 2011-12-08 03:58:54 PST
Confirming (reproduced in the current Firefox 11.0a1 nightly on Windows 7 x64) and tweaking summary. This is a regression, Firefox 8 works fine.
Comment 5 Loic 2011-12-08 05:22:28 PST
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).
Comment 6 Alice0775 White 2011-12-08 10:54:39 PST
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
Comment 7 Alice0775 White 2011-12-08 10:56:01 PST
P.S.; I think the problem step 5 of comment #5 is different bug, and maybe Bug 701662.
Comment 8 Loic 2011-12-08 11:24:24 PST
(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
Comment 9 Guillermo Moya (MetalS) 2011-12-08 17:45:48 PST
Also if you right click and use the controls of the menu the controls don't work.
Comment 10 Jared Wein [:jaws] (please needinfo? me) 2011-12-16 10:21:27 PST
*** Bug 711499 has been marked as a duplicate of this bug. ***
Comment 11 Daniel Holbert [:dholbert] 2011-12-16 10:28:00 PST
Requesting "tracking-firefox11", as this is a regression in Firefox 11 (which is migrating from Nightly to Aurora in a few days).
Comment 12 Wladimir Palant 2011-12-18 22:29:42 PST
Bug 711675 is probably related.
Comment 13 Jared Wein [:jaws] (please needinfo? me) 2011-12-20 14:49:16 PST
*** Bug 712264 has been marked as a duplicate of this bug. ***
Comment 14 Boris Zbarsky [:bz] 2011-12-21 08:08:58 PST
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?
Comment 15 Alice0775 White 2011-12-21 09:44:03 PST
*** Bug 712673 has been marked as a duplicate of this bug. ***
Comment 16 Jared Wein [:jaws] (please needinfo? me) 2011-12-21 11:33:23 PST
(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?
Comment 17 Boris Zbarsky [:bz] 2011-12-21 11:41:15 PST
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.
Comment 18 Jared Wein [:jaws] (please needinfo? me) 2011-12-21 13:10:22 PST
s/videocontrols.xml/VideoDocument.cpp in comment #16

Thanks for the pointers Boris. I can try to take this bug.
Comment 19 Justin Dolske [:Dolske] 2011-12-21 17:48:05 PST
Might bug 693206 help to avoid this? Easy enough to try, I suppose.
Comment 20 Jared Wein [:jaws] (please needinfo? me) 2011-12-21 21:25:12 PST
(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.
Comment 21 Jared Wein [:jaws] (please needinfo? me) 2011-12-22 15:03:44 PST
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 22 Jared Wein [:jaws] (please needinfo? me) 2011-12-22 15:25:50 PST
Comment on attachment 583947 [details] [diff] [review]
WIP of patch for bug 708431

Clearing review, I forgot to call SetScriptGlobalObject on the MediaDocument.
Comment 23 Jared Wein [:jaws] (please needinfo? me) 2011-12-22 15:28:13 PST
Created attachment 583953 [details] [diff] [review]
Patch for bug 708431
Comment 24 Jared Wein [:jaws] (please needinfo? me) 2011-12-22 15:30:39 PST
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 25 Boris Zbarsky [:bz] 2011-12-22 18:06:15 PST
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.
Comment 26 Jared Wein [:jaws] (please needinfo? me) 2011-12-22 19:51:06 PST
Created attachment 584000 [details] [diff] [review]
Patch for bug 708431 v1.1

Only linking the stylesheet when the script global is being set.
Comment 27 Jared Wein [:jaws] (please needinfo? me) 2012-01-06 04:08:56 PST
(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 28 Boris Zbarsky [:bz] 2012-01-06 22:37:27 PST
Comment on attachment 584000 [details] [diff] [review]
Patch for bug 708431 v1.1

r=me.  Silly holidays!
Comment 29 Jared Wein [:jaws] (please needinfo? me) 2012-01-07 18:44:54 PST
https://hg.mozilla.org/integration/fx-team/rev/ffedccc6bf24
Comment 30 Tim Taubert [:ttaubert] 2012-01-08 06:21:25 PST
https://hg.mozilla.org/mozilla-central/rev/ffedccc6bf24
Comment 31 Jared Wein [:jaws] (please needinfo? me) 2012-01-12 17:33:37 PST
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 32 Alex Keybl [:akeybl] 2012-01-17 13:42:44 PST
Comment on attachment 584000 [details] [diff] [review]
Patch for bug 708431 v1.1

[Triage Comment]
Low risk - approving for aurora.
Comment 33 Jared Wein [:jaws] (please needinfo? me) 2012-01-17 14:02:40 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/bc9d7a1e62b7
Comment 34 Mihaela Velimiroviciu (:mihaelav) 2012-02-03 07:23:40 PST
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
Comment 35 Mihaela Velimiroviciu (:mihaelav) 2012-02-03 07:25:16 PST
(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

Note You need to log in before you can comment on or make changes to this bug.