Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

VERIFIED FIXED in mozilla17

Status

()

Core
Plug-ins
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: IU, Assigned: johns)

Tracking

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Does this happen even if you don't have any add-ons enabled/installed? (In other words, is the behavior similar to bug 777238?)
(Reporter)

Comment 2

5 years ago
(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?
(Assignee)

Comment 4

5 years ago
(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.
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Depends on: 767635
OS: Windows XP → All
Hardware: x86 → All
(Assignee)

Updated

5 years ago
Status: ASSIGNED → NEW
(Assignee)

Comment 5

5 years ago
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

Updated

5 years ago
Duplicate of this bug: 783460
(Assignee)

Comment 7

5 years ago
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
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)

Updated

5 years ago
Assignee: nobody → jschoenick
Status: NEW → ASSIGNED
Depends on: 784135
(Assignee)

Updated

5 years ago
Blocks: 784135
No longer depends on: 784135

Comment 9

5 years ago
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+
(Assignee)

Comment 10

5 years ago
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
Attachment #653502 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/3adcae5fbefb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Duplicate of this bug: 784699
(Reporter)

Comment 13

5 years ago
Verifying fix with build: http://hg.mozilla.org/mozilla-central/rev/102dc2118737

Thanks
Status: RESOLVED → VERIFIED
Duplicate of this bug: 784135
Duplicate of this bug: 781662
(Assignee)

Updated

5 years ago
No longer depends on: 767635
You need to log in before you can comment on or make changes to this bug.