Closed Bug 782644 Opened 12 years ago Closed 12 years ago

Click-to-play breaks video on nbcnews.com, if Adblock Plus, NoScript, Greasemonkey, Request Policy, or Redirector is present

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla17

People

(Reporter: fehe, Assigned: johns)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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.
Does this happen even if you don't have any add-ons enabled/installed? (In other words, is the behavior similar to bug 777238?)
(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.
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?
(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.
Status: NEW → ASSIGNED
Depends on: 767635
OS: Windows XP → All
Hardware: x86 → All
Status: ASSIGNED → NEW
This hack should fix the behavior if that is what's happening here, but 767635 should fix the start/stop logic more thoroughly
We should land this so we don't need to backport 767635 to aurora
Attachment #651883 - Attachment is obsolete: true
Attachment #653502 - Flags: review?(joshmoz)
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.
Assignee: nobody → jschoenick
Status: NEW → ASSIGNED
Depends on: 784135
Blocks: 784135
No longer depends on: 784135
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.
Attachment #653502 - Flags: review?(joshmoz) → review+
https://hg.mozilla.org/mozilla-central/rev/3adcae5fbefb
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Verifying fix with build: http://hg.mozilla.org/mozilla-central/rev/102dc2118737

Thanks
Status: RESOLVED → VERIFIED
No longer depends on: 767635
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: