Last Comment Bug 726734 - plugin instances should reload when the src/data URL changes
: plugin instances should reload when the src/data URL changes
Status: RESOLVED FIXED
[qa+]
: regression, reproducible
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal with 7 votes (vote)
: mozilla14
Assigned To: Josh Aas
:
Mentors:
: 728576 733703 (view as bug list)
Depends on: 747055
Blocks: 90268
  Show dependency treegraph
 
Reported: 2012-02-13 12:24 PST by Stefan
Modified: 2012-06-14 09:58 PDT (History)
34 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
verified
verified


Attachments
fix v1.0 (1.18 KB, patch)
2012-03-21 07:30 PDT, Josh Aas
no flags Details | Diff | Splinter Review
fix v1.1 (add a test) (3.15 KB, patch)
2012-03-21 12:37 PDT, Josh Aas
jst: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Stefan 2012-02-13 12:24:44 PST
User Agent:  

Steps to reproduce:

1. Install and activate flashblock 1.5.15.1, tweak version checking if necessary.
2. Open http://www.youtube.com/watch?v=3fqBuo5JZoY
3. Click at play symbol.


Actual results:

3. Video does not start playing.


Expected results:

3. Start playing.

The first bad revision is:
changeset: 85853:15b00ab7f22d
user: Josh Aas <joshmoz@gmail.com>
date: Tue Jan 31 16:55:54 2012 -0500
summary: Bug 90268: Change plugin instance ownership from layout to content. r=roc r=bsmedberg

Regression: Bug 682951 - Youtube video does not start with flashblock
Comment 1 Virgil Dicu [:virgil] [QA] 2012-02-15 08:38:49 PST
Mozilla/5.0 (X11; Linux x86_64; rv:13.0a1) Gecko/20120207 Firefox/13.0a1
Mozilla/5.0 (Windows NT 6.1; rv:13.0a1) Gecko/20120215 Firefox/13.0a1

Confirmed with steps from comment 0.
Comment 2 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-02-15 11:22:45 PST
Josh, can you have a look at this bug? Is it a regression to your fix? This sounds like it might be a bug in the add-on but I'm not sure.
Comment 3 Steven Michaud [:smichaud] (Retired) 2012-02-15 13:14:28 PST
Please test with today's nightlies.

Please also test with one of the builds at the following URL, which have my patch for bug 723520:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-292c77354647/
Comment 5 Steven Michaud [:smichaud] (Retired) 2012-02-15 13:40:53 PST
Thanks, Stefan.
Comment 6 Steven Michaud [:smichaud] (Retired) 2012-02-15 14:01:32 PST
I can reproduce this bug, too, on OS X 10.6.8 (testing with today's mozilla-central nightly).  I won't be able to get to it for a while, though.
Comment 7 Josh Aas 2012-02-16 16:12:35 PST
A little debugging shows that we're starting the plugin instance just fine after clicking to play the video, and, as expected, we're sending an NPP_SetWindow message over IPC. Still not sure why the video doesn't play though.
Comment 8 Philip Chee 2012-02-23 09:41:59 PST
Very strange. My own self builds work:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120223 Firefox/13.0a1 
But downloaded nightlies from ftp.mozilla.org show this bug.
Comment 9 tomko222 2012-02-25 07:07:18 PST
I can confirm this bug too.
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:13.0) Gecko/20120223 Firefox/13.0a1
Comment 10 her34her34 2012-03-01 10:11:09 PST
I can confirm this bug too.

Error message:

Error: e.target.ownerDocument is undefined
Source File: chrome://flashblock/content/flashblock.js
Line: 267
Comment 11 Ozsmeg 2012-03-02 00:25:10 PST
Works fine if you add youtube.com to the flashblock whitelist.
Comment 12 tomko222 2012-03-02 10:16:39 PST
(In reply to Ozsmeg from comment #11)
> Works fine if you add youtube.com to the flashblock whitelist.

For example I use flash mainly to stop youtube autoplay so adding it to whitelist dont make sense for me...
Comment 13 xtrem123 2012-03-02 10:30:53 PST
agree with tomko222
Comment 14 Alex Keybl [:akeybl] 2012-03-04 16:57:57 PST
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:13.0) Gecko/20120301 Firefox/13.0a1

STR:

1) Install Flashblock and make sure it's enabled by default (I'm using 1.5.15.1)
2) Go to http://www.cbsnews.com/8301-3460_162-57390236/paul-limbaugh-apologized-for-personal-gain/
3) Click "PLAY CBS NEWS VIDEO"
4) Click Flashblock play button

Expected: video plays
Occurs: black box where Flash player is
Comment 15 Danial Horton 2012-03-06 05:44:35 PST
The issue is happening in reverse on dailymotion, video's are playing back when flashblock is enabled, no white listing has been set.
Comment 16 Matthias Versen [:Matti] 2012-03-08 03:47:46 PST
*** Bug 733703 has been marked as a duplicate of this bug. ***
Comment 17 Danial Horton 2012-03-08 04:04:54 PST
So whats going on with this, it is a fairly painful regression, particular on a number of tabs that can put the containers memory use past 1GB 

Any potential work around in the go for flashblock itself?, the e.target.ownerDocument is undefined is kind of sus, I haven't seen that on any of the affected sites i visit.
Comment 18 Philip Chee 2012-03-08 20:23:47 PST
Neil suggested that I might need e.originalTarget in Flashblock instead but I'll need a reproducible test case to work on.
Comment 19 Danial Horton 2012-03-08 23:34:39 PST
Error: e.target.ownerDocument is undefined
Source file: chrome://flashblock/content/flashblock.js
Line: 267

when clicking the flashblock overlay on http://www.watchanimeon.com/naruto-shippuden-episode-253/
Comment 20 Danial Horton 2012-03-08 23:39:34 PST
Error: e.target.ownerDocument is undefined
Source file: chrome://flashblock/content/flashblock.js
Line: 267

when clicking the flash overlay on http://www.youtube.com/watch?v=JLdB0WEixjM

the watchanimeon flash video loads
the youtube video doesn't :|
Comment 21 stevfe 2012-03-16 00:29:40 PDT
So is flashblock going to be fixed?

Firefox 13 is in aurora channel and problem is still there.
Comment 22 Jim Jeffery not reading bug-mail 1/2/11 2012-03-20 04:22:31 PDT
New dev version here: http://downloads.mozdev.org/flashblock/flashblock-1.5.unstable.xpi 1.5.16a2
but it does not fix the problem.

http://flashblock.mozdev.org/installation1.html
Comment 23 Josh Aas 2012-03-21 07:05:22 PDT
I did some debugging on Linux (FC16, trunk code) and here is what I think is going on.

After clicking the play button from Flashblock, the DOM node for the plugin instance is added to the document. It has a useless src/data attribute, about:home or about:blank (I don't remember). As a result of adding this node to the document we instantiate a plugin and kick off a stream for the useless src/data URI. Later on Flashblock fixes the src/data attribute on the instance DOM node. This calls nsObjectLoadingContent::LoadObject, which attempts to re-load the object since the URI is now different. The result is a call to nsObjectLoadingContent::AsyncStartPluginInstance, which returns early doing nothing because we already have an instance. Thus the real stream is never delivered to the plugin.

It would be best if the src/data attribute was set properly when the DOM node was added back to the document so the first instantiation was correct. I'm not sure if we ever supported reloading instances when the src/data URL changed, but that is the fix we'll probably have to make on our end if we need to.
Comment 24 Josh Aas 2012-03-21 07:30:55 PDT
Created attachment 607942 [details] [diff] [review]
fix v1.0
Comment 25 Philip Chee 2012-03-21 07:38:04 PDT
> After clicking the play button from Flashblock, the DOM node for the plugin instance is
> added to the document. It has a useless src/data attribute, about:home or about:blank
> (I don't remember). As a result of adding this node to the document we instantiate a
> plugin and kick off a stream for the useless src/data URI. Later on Flashblock fixes
> the src/data attribute on the instance DOM node.

I had to do this because a null src attribute causes OS/2 to crash. See the following snippet from flashblock.xml:

function flashblockSetTitle(current, placeholder, isStandalone) {
	// non-null "about:blank" value to prevent OS/2 crashing
	var fakeURI = "about:blank";
Comment 26 Stefan 2012-03-21 11:55:33 PDT
(In reply to Josh Aas (Mozilla Corporation) from comment #24)
> Created attachment 607942 [details] [diff] [review]
> fix v1.0

Works like a charm, thx!
Comment 27 Josh Aas 2012-03-21 12:37:01 PDT
Created attachment 608057 [details] [diff] [review]
fix v1.1 (add a test)
Comment 28 Josh Aas 2012-03-22 11:02:46 PDT
try server run:

https://tbpl.mozilla.org/?tree=Try&rev=c1b7e9a3b7ed
Comment 29 Johnny Stenback (:jst, jst@mozilla.com) 2012-03-22 12:15:14 PDT
Comment on attachment 608057 [details] [diff] [review]
fix v1.1 (add a test)

Stealing review, r=jst
Comment 30 Jim Jeffery not reading bug-mail 1/2/11 2012-03-22 14:58:18 PDT
Try build fixed the issue for me.  Youtube, and the cbs new link from comment #14 now work as expected. 

Tested with Flashblock 1.5.16a2 latest 'unstable' developers build on win7 x64, win32 build
Comment 31 Danial Horton 2012-03-22 15:24:44 PDT
i don't have time to check myself (my line is in the process of being repaired)
but can someone verify the chia player, dailymotions on site player (not the embed one) and the flash uploader for dailymotion?
Comment 32 Josh Aas 2012-03-23 05:34:27 PDT
pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/d85d2f90b632
Comment 33 Josh Aas 2012-03-23 06:10:09 PDT
Comment on attachment 608057 [details] [diff] [review]
fix v1.1 (add a test)

[Approval Request Comment]
Regression caused by (bug #): 90268
User impact if declined: Flasblock broken on high profile sites (YouTube), other web compat issues w/o Flashblock.
Testing completed (on m-c, etc.): Automated test checked in.
Risk to taking this patch (and alternatives if risky): 4/10
String changes made by this patch: None
Comment 34 Ed Morley [:emorley] 2012-03-24 14:11:54 PDT
https://hg.mozilla.org/mozilla-central/rev/d85d2f90b632
Comment 35 Ahmed Aly 2012-03-24 14:16:00 PDT
if this patch works without regressions i guess it should be back ported to gecko 13
Comment 36 yiaghyku 2012-03-26 12:01:33 PDT
I guess the problem was with firefox. I tried looking at the patches to see what was wrong but couldn't make sense of it.

Would someone explain to a non-programmer what the problem was and how the patch fixes it?
Comment 37 Ryan VanderMeulen [:RyanVM] 2012-03-26 12:07:03 PDT
Comments #23 and #25 explain the issue.
Comment 38 yiaghyku 2012-03-26 12:31:03 PDT
Yes i've read the thread.

From what i can tell when the user wants to unblock the flash object, the flashblock addon changes the flash object address to trigger a flash object refresh from browser.

This has always worked in firefox until firefox 13. So i'm wondering what happened in firefox 13 that broke flashblock and how does the patch fix it?
Comment 39 IU 2012-03-26 14:28:41 PDT
(In reply to yiaghyku from comment #38)
> So i'm wondering what happened in firefox 13 that broke flashblock...

Read bug 90268
Comment 40 Alex Keybl [:akeybl] 2012-03-26 15:39:29 PDT
Comment on attachment 608057 [details] [diff] [review]
fix v1.1 (add a test)

[Triage Comment]
Since the risk evaluation here does not yet warrant considering backing out bug 90268, and this bug is critical to fix for our Flashbock users, let's land on Aurora 13.
Comment 41 Alex Keybl [:akeybl] 2012-03-26 15:40:17 PDT
If you could also add the qawanted keyword with any testing that we can do to lower that 4/10 risk rating, it'd be greatly appreciated :)
Comment 42 Josh Aas 2012-03-27 00:59:31 PDT
pushed to mozilla-aurora

http://hg.mozilla.org/releases/mozilla-aurora/rev/5c4189019bc1
Comment 43 Josh Aas 2012-04-10 06:23:24 PDT
*** Bug 728576 has been marked as a duplicate of this bug. ***
Comment 44 Simona B [:simonab ] -PTO- back Sept 5th 2012-05-08 08:10:00 PDT
Verified that YouTube and cbs videos play as expected on Firefox 13 beta 2 when Flashblock 1.5.15.1 and Flashblock 1.5.16a2 are installed.

Tested on Windows 7, Ubuntu 11.10 and Mac OS x 10.6 using the STR from the description and from Comment 14.

Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0
Comment 45 Simona B [:simonab ] -PTO- back Sept 5th 2012-06-14 09:58:50 PDT
Verified on Firefox 14 beta 7 that the videos from YouTube and CBS play properly - used the STR from the Description while Flashblock 1.5.15.1 and Flashblock 1.5.16a2 were installed.

Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:14.0) Gecko/20100101 Firefox/14.0

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