Some Flash videos no longer play in presence of JavaScript-based content policies

RESOLVED FIXED in mozilla2.0b7

Status

()

defect
P1
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: gaubugzilla, Assigned: bzbarsky)

Tracking

({regression, relnote})

Trunk
mozilla2.0b7
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 beta8+, status1.9.2 unaffected)

Details

()

Attachments

(3 attachments, 3 obsolete attachments)

Some Flash videos will no longer play in Minefield if Adblock Plus is installed, e.g. the video on http://edition.cnn.com/ - clicking "Click to play" button makes the image disappear but the Flash video that should display instead doesn't show up. Same happens with RuTube videos like http://rutube.ru/tracks/3674261.html - here the video should show up on page load but doesn't.

This regressed in Minefield between 20100913 and 20100914 (reproduced on Windows 7 x64). Adblock Plus doesn't need to be installed - it is also reproducible on a clean profile with the minimal content policy (attached, doesn't do anything but dump URLs coming through content policies to console). Which certainly means that NoScript, GreaseMonkey & Co. are affected as well.

Will try to narrow down the regression range with hourly builds though I already have a pretty good idea where this regressed,
Regression range is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1b3947ed93c6&tochange=29c228a4d7eb, so this is most certainly fallout from JavaScript compartments.
Blocks: compartments
blocking2.0: --- → ?
Keywords: regression
http://flashbuilder.eu/flash-player-version.html is another website that is affected, fails to display version number of the installed Flash plugin. This page is simpler if somebody needs a testcase.
Confirming....
Can you see any error messages in the JS console?
On flashbuilder.eu - no, nothing but a bunch of Analytics-related warnings.
This is different from bug 594482, which is Java but much older?
Here are other sample:

http://www.ardmediathek.de/ard/servlet/
http://www.zdf.de/ZDFmediathek/hauptnavigation/startseite/#/hauptnavigation/startseite

If i deactivate adblock on addon manager the sites work

Also the windowsmediaplayer as another player don't work on this site, if adblock activated...
Same happens with Vkontakte videos like
http://vkontakte.ru/video5509074_151026486
(In reply to comment #6)
> This is different from bug 594482, which is Java but much older?

Yes, definitely different. This bug is about a recent regression, caused by compartments landing, bug 594482 has been reported a while ago however.
What I mean is: does any JS content policy trigger this bug, even the no-op one from that bug? If so, the two bugs are probably very much related and we have a smaller testcase to deal with.
Yes, the two bugs have the same no-op content policy attached - and both are reproducible with it. And they probably both have something to do with wrappers created at the wrong time (as such bugs typically do). But I would be very surprised if the cause is the same in both cases.
OS: Windows 7 → All
I cannot see the video in coment 8 nor the videos at:

http://www.cnet.com
http://www.nasa.gov
I cannot see the install button on http://www.tweetdeck.com/desktop/
#14: The "download now, its free" button? Do you have adblock installed?
(In reply to comment #15)
> #14: The "download now, its free" button? Do you have adblock installed?

My bad, just checked with ABP disabled and it shows. However enabled with the page white-listed completely it still doesn't show. Not sure if that info helps this bug or not though.
Just wanted to add that this bug also partially breaks the Add-on Stylish. As a result of all the 'breakage' I went back to cset 3c762301b719 and everything that wasn't working now does.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101013 Firefox/4.0b8pre - Build ID: 20101013173441
I can confirm that the download button does not show up as long as ABP is enabled. As soon as it is disabled the button will show.

I think this problem appears on any site using swfobject to display Flash content. Simply creating a Flash object via JavaScript using createElement does not cause the bug to appear. So it must be something else in the swfobject code that makes this bug appear.

Source-code for swfobject can be downloaded here:
http://code.google.com/p/swfobject/downloads/list
Posted file Greasemonkey Script workaround (obsolete) —
The problem occurs if the Flash movie is loaded through the data attribute of the object element. If it loads the Flash movie through the embed element it will show up.

I've created a Greasemonkey script which should work around the issue until a proper patch is available.

The script will only add an embed element if no such element is present in the object element. It will also append the flashvars attribute to the embed source url if it is present.

I think this will fix most broken Flash movies, but I'm not sure.
bz, what does your blocker list look like?
Assignee: nobody → bzbarsky
blocking2.0: ? → betaN+
Posted file Updated Greasemonkey Script (obsolete) —
Some object tags hadn't been created after the 500ms delay so I upped it to 800ms, in addition I've made it run a second time 5 seconds after the first one is done, that should catch any late objects. 

I've tried using DOMSubtreeModified on some other scripts but I got quite a high CPU load. Any cleaner way to get it done?
Attachment #483937 - Attachment is obsolete: true
Some Flash objects did not load (such as YouTube embed), because the embed-element required the type-attribute to be present.

I also tried again with DOMSubtreeModified and now I can't see any increase in CPU usage.
Attachment #483943 - Attachment is obsolete: true
Whiteboard: relnote
> bz, what does your blocker list look like?

Nasty, brutish, and short.  ;)

I was meaning to look into this anyway.
The sites listed in this post are also affected and do not show videos if ABP+ is installed.  The other posts in the thread have a GreaseMonkey hack to make things work: 

http://forums.mozillazine.org/viewtopic.php?p=10029773#p10029773
(In reply to comment #18)
> I can confirm that the download button does not show up as long as ABP is
> enabled. As soon as it is disabled the button will show.

In my case, I had to remove AdBlock Plus (version 1.2.2) completely. Disabling wasn't enough. Sigh....
Until this bug land or is fixed, Minefield in rendered useless. Cannot see any flash content that is embedded. Most Major news outlets/site are no longer viewable.
(In reply to comment #26)
> Until this bug land or is fixed, Minefield in rendered useless. Cannot see any
> flash content that is embedded. Most Major news outlets/site are no longer
> viewable.

First, please avoid evangelism in bugs. This comment adds no new information.
Second, disable or uninstall AdBlock Plus, and you should be fine.
It looks like the problem is two things. First of all we end up trying to get valueOf on the Xray for converting to string. Second we now call NewResolve more (we store the properties defined on the Xray on its holds), and the second call to NewResolve ends up calling nsObjectFrame::PrepareInstanceOwner which calls StopPluginInternal. More tomorrow.
(In reply to comment #28)
>  First of all we end up trying to get
> valueOf on the Xray for converting to string.

This is likely an artifact of the dump() call in the minimal test content policy, which logs the node (context) parameter:

dump("shouldLoad: " + contentType + " " +
                          (contentLocation ? contentLocation.spec : "null") + " " +
                          (requestOrigin ? requestOrigin.spec : "null") + " " +
                          node + " " +
                          mimeTypeGuess + "\n");

I guess real world content policies don't log like this by default.
Yes, that's why I pointed out that there's two problems. Removing the dump and getting .ownerDocument results in the same problem.
We end up with two separate plugin instance owners here.  The first is set up by nsObjectFrame::PrepareInstanceOwner in response to OnStartRequest.  The second is set up by the same in response to nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe.  The second is what ends up set up correctly, but the first is what we try to pump data into.

In fact, the second is set up while trying to set up the first:

#5  0x009b725c in nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe (wrapper=0x1d1e1380, obj=0x1f0887f8, _result=0xbfffa63c) at /Users/bzbarsky/mozilla/vanilla/mozilla/dom/base/nsDOMClassInfo.cpp:9385
#6  0x009bd6a4 in nsHTMLPluginObjElementSH::NewResolve (this=0x21a64b30, wrapper=0x1d1e1380, cx=0x61236b0, obj=0x1f0887f8, id={asBits = 106957312}, flags=1, objp=0xbfffa6dc, _retval=0xbfffa6e0) at /Users/bzbarsky/mozilla/vanilla/mozilla/dom/base/nsDOMClassInfo.cpp:9744
...
#29 0x005cf853 in nsContentPolicy::ShouldLoad (this=0x4d9e8b0, contentType=5, contentLocation=0x25803880, requestingLocation=0x1d1d2330, requestingContext=0x25803684, mimeType=@0xbfffc13c, extra=0x0, decision=0xbfffc158) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsContentPolicy.cpp:218
#30 0x00208d4e in NS_CheckContentLoadPolicy (contentType=5, contentLocation=0x25803880, originPrincipal=0x1d1d2050, context=0x25803684, mimeType=@0xbfffc13c, extra=0x0, decision=0xbfffc158, policyService=0x0, aSecMan=0x0) at nsContentPolicyUtils.h:223
#31 0x011bd7cc in nsPluginHost::InstantiateEmbeddedPlugin (this=0x4e240b0, aMimeType=0x216893a8 "application/x-shockwave-flash", aURL=0x25803880, aOwner=0x21644b20) at /Users/bzbarsky/mozilla/vanilla/mozilla/modules/plugin/base/src/nsPluginHost.cpp:1004
...
#32 0x011b04e2 in nsPluginStreamListenerPeer::OnStartRequest (this=0x1d1d0b30, request=0x1d1e1a44, aContext=0x0) at /Users/bzbarsky/mozilla/vanilla/mozilla/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp:588
#33 0x00690011 in nsObjectLoadingContent::OnStartRequest (this=0x25803620, aRequest=0x1d1e1a44, aContext=0x0) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsObjectLoadingContent.cpp:738

The fact that we have that second load to start with is bug 484992.
If I change the content policy shouldLoad implementation by adding this line:

  dump(node.myExpando + "\n");

then I get exactly the same failure symptoms on 1.9.2 as I'm getting on trunk: we end up going through XPC_NW_NewResolve for "myExpando" and land in the classinfo code, etc.  But for some reason we do NOT hit XPC_NW_NewResolve for node.ownerDocument, though we _do_ hit XPC_NW_GetOrSetProperty.  I have no idea how we're managing that, yet.
So at the moment, I think the obvious options are:

1) Fix bug 484992
2) Change the classinfo resolve hook here to not resolve things on wrappers
3) Make the resolve behavior match 1.9.2, however that worked.
> then I get exactly the same failure symptoms on 1.9.2 as I'm getting on trunk:
> we end up going through XPC_NW_NewResolve for "myExpando" and land in the
> classinfo code, etc.  But for some reason we do NOT hit XPC_NW_NewResolve for
> node.ownerDocument, though we _do_ hit XPC_NW_GetOrSetProperty.  I have no idea
> how we're managing that, yet.

On trunk with revision edb0506b0c23 (just before compartments) I see XPC_NW_NewResolve for ownerDocument, and we have the exact same issue (two plugin instance owners). The only "regression" is that if you just dump the node we used to not get into NewResolve. We should really fix the two plugin instance owners.
As in, you don't see the issue with the minimal testcase pre-compartments, but do see it on hat flashbuilder link?

fwiw, I did check that the other two urls in comment 0 are also using <object>, not <embed>, to embed flash.
(In reply to comment #37)
> As in, you don't see the issue with the minimal testcase pre-compartments, but
> do see it on hat flashbuilder link?

Right.
That's ... quite odd.  ;)  The minimal testcase just has the DOM that flashbuilder generates for the <object>...
Oh, I bet ownerDocument isn't hitting NewResolve for me on the minimal testcase is that the shouldLoad call for the _original_ data load resolved it already.  OK.  I'll try just fixing bug 484992.
Depends on: 484992
The patch I'm about to post in bug 484992 fixes all the testcases mentioned in this bug, when using the attached minimal policy, whether as-is or with the modifications I mentioned in comment 33.
For those who don't want to wait for the patch to get pushed I've updated the earlier GM script which should work on just about every Flash object. The previous script had some issues on some sites.
Attachment #483951 - Attachment is obsolete: true
blocking2.0: betaN+ → beta8+
Keywords: relnote
Whiteboard: relnote
(In reply to comment #42)
> Created attachment 484714 [details]
> GM workaround script, final try
> 
> For those who don't want to wait for the patch to get pushed I've updated the
> earlier GM script which should work on just about every Flash object. The
> previous script had some issues on some sites.

script works greart for greasmonkey.
I have x86_64 latrest build of firefox and thisd bug is also in it. The greasmonkey workaound works for x86_64 version also. The bug is not just in x86.
Duplicate of this bug: 606529
as mentioned here: https://bugzilla.mozilla.org/show_bug.cgi?id=606529#c3 I must disable both AB+ and NS to get a flash video running on a site where it wouldn't run otherwise. I insist: both must be disabled here.
Yes, yes.  They both use a JS-implemented content policy.  Now can we please stop adding more noise to this bug?  ;)
Duplicate of this bug: 606456
Priority: -- → P1
Fixed by checkin for bug 484992.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Latest tracemonkey does not have the fix for x86-64 and I am using windows 7 ultimate x64.
What does comment 50 have to do with this bug?
the bug is also in the 64 bit version.
The fix for this bug has landed on mozilla-central, not Tracemonkey. It will not be in Tracemonkey branch for at least a week - probably longer.

Is there a way to lock bugs to further comments? I would suggest it be used here if so. BTW, this fixed my Farmville/Mafia Wars.
Just to clarify comment 50 and comment 52

Some people have gotten into the [bad] habit of using Win64 builds from latest-tracemonkey rather than latest-trunk, because latest-trunk doesn't reliably have nightly builds.
Central don't have the x86-64 bit version if it has the fix because it was uploaded on the 20th not not the 22nd for x86-64 for windows. I don't use the 32 bit version I use the 64 bit version only.
> Is there a way to lock bugs to further comments?

No.  Else I would have used it around comment 47.

Rob, see comment 53.  This is fixed on m-c; it'll be fixed in tracemonkey the next time there's a merge from m-c to tracemonkey.
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.