Last Comment Bug 782644 - Click-to-play breaks video on nbcnews.com, if Adblock Plus, NoScript, Greasemonkey, Request Policy, or Redirector is present
: Click-to-play breaks video on nbcnews.com, if Adblock Plus, NoScript, Greasem...
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: mozilla17
Assigned To: John Schoenick [:johns]
:
:
Mentors:
http://www.nbcnews.com/
: 781662 783460 784699 (view as bug list)
Depends on:
Blocks: click-to-play 784135
  Show dependency treegraph
 
Reported: 2012-08-14 07:22 PDT by IU
Modified: 2012-09-04 13:34 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Change load blocker around in nsObjectLoadingContent (part of bug 767635 fixes) (1.52 KB, patch)
2012-08-14 14:15 PDT, John Schoenick [:johns]
no flags Details | Diff | Splinter Review
Prevent against nsObjectLoadingContent re-entry from content policy (3.75 KB, patch)
2012-08-20 13:50 PDT, John Schoenick [:johns]
jaas: review+
john: checkin+
Details | Diff | Splinter Review

Description IU 2012-08-14 07:22:10 PDT
Click-to-play results in a blank video player with only audio playing, if either Adblock Plus, NoScript, Greasemonkey, Request Policy, or Redirector is present.

STR:

1. Start Firefox with a new profile
2. Enable click-to-play
3. Install any combination of Adblock Plus, NoScript, Greasemonkey, Request Policy, or Redirector
4. Restart browser and visit http://www.nbcnews.com/
5. Click any video link or thumbnail that causes the video player to open as a popup
6. Notice that audio plays but no video is visible.
Comment 1 David Keeler [:keeler] (use needinfo?) 2012-08-14 09:34:34 PDT
Does this happen even if you don't have any add-ons enabled/installed? (In other words, is the behavior similar to bug 777238?)
Comment 2 IU 2012-08-14 10:27:59 PDT
(In reply to David Keeler from comment #1)
> Does this happen even if you don't have any add-ons enabled/installed? (In
> other words, is the behavior similar to bug 777238?)

I have 66 add-ons enabled that do not cause the problem.  Only any of the above listed set of add-ons (e.g. Adblock Plus, NoScript, Greasemonkey, Request Policy, or Redirector) cause the problem.

The behavior is consistently reproducible, so it is not the same as bug 777238.

If you're not able to reproduce, maybe I have an additional setting necessary to reproduce.  For one, I have HA disabled.
Comment 3 David Keeler [:keeler] (use needinfo?) 2012-08-14 14:04:18 PDT
John, I think what's happening is these addons are registering a content policy that gets checked when nsObjectLoadingContent::LoadObject is called. The problem is that these policies cause a js object to be created, which eventually calls nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe, which calls nsObjectLoadingContent::SyncStartPluginInstance. This function seems to assume that it's okay to load the plugin (which it's not, if click-to-play is enabled), so the plugin gets loaded even before the checks to see if it's okay to load are completed (hence the audio; I don't know what's going on with the video - I can't replicate that part). Thoughts?
Comment 4 John Schoenick [:johns] 2012-08-14 14:09:31 PDT
(In reply to David Keeler from comment #3)
> John, I think what's happening is these addons are registering a content
> policy that gets checked when nsObjectLoadingContent::LoadObject is called.
> The problem is that these policies cause a js object to be created, which
> eventually calls nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe, which
> calls nsObjectLoadingContent::SyncStartPluginInstance. This function seems
> to assume that it's okay to load the plugin (which it's not, if
> click-to-play is enabled), so the plugin gets loaded even before the checks
> to see if it's okay to load are completed (hence the audio; I don't know
> what's going on with the video - I can't replicate that part). Thoughts?

I have seen this exact behavior - we re-enter into LoadObject through content policy, but at that point mType == eType_Plugin (even though it would fail over to eType_Null later in the call). I'm going to have a patch for bug 767635 in the next day or so here that fixes this re-entrance, and greatly simplifies the start/stop plugin mess, so hopefully that will resolve this.
Comment 5 John Schoenick [:johns] 2012-08-14 14:15:29 PDT
Created attachment 651883 [details] [diff] [review]
Change load blocker around in nsObjectLoadingContent (part of bug 767635 fixes)

This hack should fix the behavior if that is what's happening here, but 767635 should fix the start/stop logic more thoroughly
Comment 6 alex_mayorga 2012-08-17 06:12:01 PDT
*** Bug 783460 has been marked as a duplicate of this bug. ***
Comment 7 John Schoenick [:johns] 2012-08-20 13:50:11 PDT
Created attachment 653502 [details] [diff] [review]
Prevent against nsObjectLoadingContent re-entry from content policy

We should land this so we don't need to backport 767635 to aurora
Comment 8 Tony Mechelynck [:tonymec] 2012-08-20 14:27:56 PDT
DISCLAIMER: The following is not a fix, just an imperfect workaround.

If you temporarily toggle plugins.click_to_play to false in about:config (assuming you've set it to true beforehand, e.g. in user.js), reloading a tab will enable any plugins on this tab regardless of this bug. Not on other tabs, but if you have, let's say, Flash and Java both enabled in about:addons you cannot (by this method) start a Flash video while blocking any Java applets in the same tab (see bug 746374 and bug 777341 for that). But it's a start. You may want to toggle the pref back afterwards.
Comment 9 Josh Aas 2012-08-21 13:13:02 PDT
Comment on attachment 653502 [details] [diff] [review]
Prevent against nsObjectLoadingContent re-entry from content policy

Review of attachment 653502 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with log message fixed.

::: content/base/src/nsObjectLoadingContent.cpp
@@ +687,1 @@
>      LOG(("OBJLC [%p]: Refusing to instantiate non-plugin, "

This log message doesn't necessarily make sense with your change to the conditions for it.
Comment 10 John Schoenick [:johns] 2012-08-21 16:47:29 PDT
Comment on attachment 653502 [details] [diff] [review]
Prevent against nsObjectLoadingContent re-entry from content policy

https://hg.mozilla.org/integration/mozilla-inbound/rev/3adcae5fbefb

try:
https://tbpl.mozilla.org/?tree=Try&rev=a16bc60ce498
Comment 11 Ed Morley [:emorley] 2012-08-22 02:45:06 PDT
https://hg.mozilla.org/mozilla-central/rev/3adcae5fbefb
Comment 12 Benjamin Smedberg [:bsmedberg] 2012-08-22 11:42:47 PDT
*** Bug 784699 has been marked as a duplicate of this bug. ***
Comment 13 IU 2012-08-22 18:05:54 PDT
Verifying fix with build: http://hg.mozilla.org/mozilla-central/rev/102dc2118737

Thanks
Comment 14 David Keeler [:keeler] (use needinfo?) 2012-08-23 09:41:11 PDT
*** Bug 784135 has been marked as a duplicate of this bug. ***
Comment 15 David Keeler [:keeler] (use needinfo?) 2012-08-28 13:24:57 PDT
*** Bug 781662 has been marked as a duplicate of this bug. ***

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