Random, unpredictable behavior with multiple YouTube embeds

RESOLVED FIXED in Firefox 20

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: rkuchiki, Assigned: tnikkel)

Tracking

({regression, reproducible})

17 Branch
mozilla22
x86_64
All
regression, reproducible
Points:
---
Bug Flags:
in-qa-testsuite ?

Firefox Tracking Flags

(firefox18- affected, firefox19+ wontfix, firefox20+ verified, firefox21+ verified, firefox22 verified, firefox-esr17-)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.52 Safari/537.17

Steps to reproduce:

Visit any page on WXVids.org that renders more than one YouTube element on a single page. For example, http://www.wxvids.org/archives/author/russell-hande

Other info:
Firefox 32-bit 18.0.1 Windows, running on 64-bit Windows 7.
Flash Player:
    File: NPSWF32_11_5_502_146.dll
    Version: 11.5.502.146
    Shockwave Flash 11.5 r502


Actual results:

It appears to be a random roulette. Sometimes they all load, most of the time some of them render a black element (but right clicking still gives you the YouTube player context menu).

On the elements that do load, the flash player may or may not operate correctly. The Play button may or may not work, and most of the time the status bar of the YouTube player is not accessible (such as volume, full screen, video quality).

This does not appear to happen if there is only 1 player on a page, and then Firefox behaves as desired.


Expected results:

All 5 video players should load and render correctly, without any issues.
When tested on the same machine with the following browsers, the desired effect occurs:

Internet Explorer 9 32-bit
Safari 5.1.7
Chrome 24.0.1312.52

Comment 1

5 years ago
I have seen this same issue with multiple YouTube embedded vids in FireFox. I didn't test Safari, but IE and Chrome both worked as expected for me as well.

Comment 2

5 years ago
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/22288130fea2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120813211942
Bad:
http://hg.mozilla.org/mozilla-central/rev/d9183f015df8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120814055342
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=22288130fea2&tochange=d9183f015df8

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/303b75594832
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120813210542
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/4ddd9c731adc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120813210943
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=303b75594832&tochange=4ddd9c731adc

Triggered by: Bug 775965
Blocks: 775965
Status: UNCONFIRMED → NEW
status-firefox18: --- → affected
tracking-firefox19: --- → ?
tracking-firefox20: --- → ?
tracking-firefox21: --- → ?
tracking-firefox-esr17: --- → ?
Component: Untriaged → Layout
Ever confirmed: true
Keywords: regression
Product: Firefox → Core

Updated

5 years ago
tracking-firefox18: --- → ?
Version: 18 Branch → 17 Branch

Updated

5 years ago
OS: Windows 7 → All

Comment 3

5 years ago
If the iframe is located outside of visible resion of current view, it seems to fail to load Flash content.

Comment 4

5 years ago
roc - can you take a look? I would have sent this over to cpearce, but I understand he's in b2g-land. This wouldn't block FF19's release, but we would accept a low risk uplift up until this coming Tuesday (1/29).
Assignee: nobody → roc
tracking-firefox18: ? → -
tracking-firefox19: ? → +
tracking-firefox20: ? → +
tracking-firefox21: ? → +
tracking-firefox-esr17: ? → -
Keywords: reproducible
Assignee: roc → cpearce

Comment 5

5 years ago
In local build
Last Good: 9c75f0428f8a
First Bad: bcac58cbf328

Regressed by:
  bcac58cbf328	Chris Pearce — Bug 775965 - Ensure presentation persists across nsSubDocumentFrame reframes. r=roc
Created attachment 707962 [details]
Slightly more reduced testcase

This is a slightly more reduced testcase... I say "slightly", because the page invokes Googles Website Translator, which then runs a bunch of obfuscated JS. That's not the easiest thing to minimize...
(Reporter)

Comment 7

5 years ago
I've disabled the Translator widget, please advise if this helps, and when I can re--enable it.
Disabling the Translator widget means the bug goes away on your site, which will probably make your users happier! We have the reduced testcase now, so disabling the Translator widget on your page won't affect our ability to fix the bug.

Comment 9

4 years ago
Chris - what are the next steps here? We'd like to resolve for FF20, considering this has missed FF19 and regressed in FF18.
status-firefox19: --- → wontfix
Assignee: cpearce → ajones
(Assignee)

Updated

4 years ago
Assignee: ajones → tnikkel
(Assignee)

Comment 10

4 years ago
Created attachment 719288 [details] [diff] [review]
patch

We reconstruct the whole document including iframes containing plugins without widgets. When we reconstruct we pick up the cached presentation and use nsObjectFrame::EndSwapDocShells to fix up the plugins. But we only register with the root prescontext if we have a widget. We need to do it even if we don't so that nsObjectFrame::DidSetWidgetGeometry gets called.

I just copied what we do in nsObjectFrame::PrepForDrawing, where we only don't register on mac when we don't have a widget.
Attachment #719288 - Flags: review?(roc)
(Assignee)

Comment 11

4 years ago
Comment on attachment 719288 [details] [diff] [review]
patch

Mats could review this too. Whoever gets to it first.
Attachment #719288 - Flags: review?(matspal)
Comment on attachment 719288 [details] [diff] [review]
patch

> I just copied what we do in nsObjectFrame::PrepForDrawing, where we only
> don't register on mac when we don't have a widget.

That seems reasonable, but it looks like EndSwapDocShells is a static method
so how does "if (mWidget)" compile?

Also, looking at nsObjectFrame::CallSetWindow there's this:

#ifdef XP_MACOSX
  nsWeakFrame weakFrame(this);
  mInstanceOwner->FixUpPluginWindow(nsPluginInstanceOwner::ePluginPaintDisable);
  if (!weakFrame.IsAlive()) {
    return NS_ERROR_NOT_AVAILABLE;
  }
#endif

so 'objectFrame' might be deleted in the code you added.
Attachment #719288 - Flags: review?(matspal) → review-
(Assignee)

Updated

4 years ago
Attachment #719288 - Flags: review?(roc)
(Assignee)

Comment 13

4 years ago
(In reply to Mats Palmgren [:mats] from comment #12)
> That seems reasonable, but it looks like EndSwapDocShells is a static method
> so how does "if (mWidget)" compile?

I was testing on non-mac so I originally just made the call unconditional. When I went make the final patch I added in the mac case and assumed I had done it right, but I forgot the static part.

> Also, looking at nsObjectFrame::CallSetWindow there's this:
> 
> #ifdef XP_MACOSX
>   nsWeakFrame weakFrame(this);
>  
> mInstanceOwner->FixUpPluginWindow(nsPluginInstanceOwner::
> ePluginPaintDisable);
>   if (!weakFrame.IsAlive()) {
>     return NS_ERROR_NOT_AVAILABLE;
>   }
> #endif
> 
> so 'objectFrame' might be deleted in the code you added.

Good catch. This was a problem before my patch too. I'll just fix it though.
(Assignee)

Comment 14

4 years ago
Created attachment 719577 [details] [diff] [review]
patch v2
Attachment #719288 - Attachment is obsolete: true
Attachment #719577 - Flags: review?(matspal)
Comment on attachment 719577 [details] [diff] [review]
patch v2

Looks good, r=mats
Attachment #719577 - Flags: review?(matspal) → review+
(Assignee)

Comment 16

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4c21d0df646
(Assignee)

Comment 17

4 years ago
Assuming we don't find any problems with this in a few days this should be safe enough for aurora and beta.
https://hg.mozilla.org/mozilla-central/rev/d4c21d0df646
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(Assignee)

Comment 19

4 years ago
Comment on attachment 719577 [details] [diff] [review]
patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug  	775965
User impact if declined: plugins won't show in some circumstances
Testing completed (on m-c, etc.): been on mc for several days
Risk to taking this patch (and alternatives if risky): low risk fix
String or UUID changes made by this patch: none
Attachment #719577 - Flags: approval-mozilla-beta?
Attachment #719577 - Flags: approval-mozilla-aurora?
status-firefox20: --- → affected
status-firefox21: --- → affected
status-firefox22: --- → fixed
Comment on attachment 719577 [details] [diff] [review]
patch v2

Low risk and early enough in FF20 beta, we can take this uplift.
Attachment #719577 - Flags: approval-mozilla-beta?
Attachment #719577 - Flags: approval-mozilla-beta+
Attachment #719577 - Flags: approval-mozilla-aurora?
Attachment #719577 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 21

4 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/ecd6788ad15a
https://hg.mozilla.org/releases/mozilla-beta/rev/a3132e813f52
status-firefox20: affected → fixed
status-firefox21: affected → fixed
Verified fixed using the testcase on FF 20b3 on Win 7, Ubuntu 12.04 and Mac OS X 10.8.2
status-firefox20: fixed → verified
Flagging for verification in Firefox 21 and 22.

Henrik, do you think this could have a Mozmill testcase?
Flags: in-qa-testsuite?(hskupin)
Keywords: verifyme
Verified fixed using the testcase on FF 21b3 on Win 7, Ubuntu 12.04 and Mac OS X 10.8.3
status-firefox21: fixed → verified

Comment 25

4 years ago
Verified as fixed on Firefox 22 beta 6 (20130617145905) on Windows 7 64bit, Ubuntu 12.10 64bit and Mac OSX 10.8.2.
status-firefox22: fixed → verified
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Removing my name from in-qa-testsuite flag for a better query.
Flags: in-qa-testsuite?(hskupin) → in-qa-testsuite?
You need to log in before you can comment on or make changes to this bug.