No showing click-to-play doorhanger icon on some pages

VERIFIED FIXED in mozilla15

Status

()

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

People

(Reporter: tetsuharu, Assigned: jaws)

Tracking

Trunk
mozilla15
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1
Build ID: 20120505030510

Steps to reproduce:

[Environment]
http://hg.mozilla.org/mozilla-central/rev/0a48e6561534

1. enable click to play
2. visit this page: http://www.slideshare.net/haricot/how-browser-engines-work/
3. Flip a slide to 60.


Actual results:

The page loads an embedded YouTube video with flash plugin.
But browser does not showing click-to-play doorhanger icon.
(Reporter)

Updated

5 years ago
Blocks: 711552
OS: Windows 7 → All
Hardware: x86_64 → All

Updated

5 years ago
Blocks: 738698
Happen the following error When occurring this bug:
Timestamp: 2012/05/06 1:07:45
Error: TypeError: overlay is null
Source File: chrome://browser/content/browser.js
Line: 10711

And I found a strange thing. This bug does not occur when I set breakpoints to the above point with Venkman & stop with breakpoint. This bug may be related to the timing of structuring DOM.
Stack trace from Venkman:
#0: function PH_handleClickToPlayEvent(aPlugin=function:) in <chrome://browser/content/browser.js> line 10710
#1: function anonymous() in <chrome://browser/content/browser.js> line 10608
There are cases where the overlay can be null (such as if the element is display:none when the element is added to the DOM). Mobile checks for this and we're not, so we need to add a null check in PH_handlieClickToPlayEvent.

Thanks saneyuki_s for finding this.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Created attachment 622190 [details] [diff] [review]
Patch for bug

Added a null-check for the overlay.
Attachment #622190 - Flags: review?(felipc)
This will not fix the problem that clicking the overlay won't work after it's made visible, right? did mobile come up with any solution for that?
(In reply to Felipe Gomes (:felipe) from comment #5)
> This will not fix the problem that clicking the overlay won't work after
> it's made visible, right? did mobile come up with any solution for that?

AFAIK, they didn't come up with a solution for this. See bug 741130 for the mobile version of this case.
(In reply to Felipe Gomes (:felipe) from comment #5)
> This will not fix the problem that clicking the overlay won't work after
> it's made visible, right? did mobile come up with any solution for that?

If we fix this bug by the approach of attachment 622190 [details] [diff] [review], we may need to add the context menu to play & permit plugins & permit on Click-to-play placeholder.
(In reply to OHZEKI Tetsuharu [:saneyuki_s] from comment #7)
> If we fix this bug by the approach of attachment 622190 [details] [diff] [review]
> [review], we may need to add the context menu to play & permit plugins &
> permit on Click-to-play placeholder.

Why do we need that, Ohzeki? you mean because of bug 741130? We are planning on dealing with that on a separate bug
Comment on attachment 622190 [details] [diff] [review]
Patch for bug

Review of attachment 622190 [details] [diff] [review]:
-----------------------------------------------------------------

So this bug is just about not throwing an exception here that prevents the function to continue and the doorhanger from being shown. We should investigate on another bug how to fix bug 741130, probably by having the click handler directly on the XBL as we talked in person

::: browser/base/content/browser.js
@@ +7277,1 @@
>  

please add braces around the `if (overlay)` because this spans more than 1 line
Attachment #622190 - Flags: review?(felipc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/f77082549e0e
Flags: in-testsuite-
Target Milestone: --- → mozilla15
(In reply to Felipe Gomes (:felipe) from comment #8)
> Why do we need that, Ohzeki? you mean because of bug 741130? We are planning
> on dealing with that on a separate bug

The patch of this bug cannot add the event listener to a placeholder when |overlay| is undefined. In such case, user cannot only play the plugin which is in such case. (This related to bug 742753.)
I'll talk about this at bug 741130 or another new bug.
(In reply to OHZEKI Tetsuharu [:saneyuki_s] from comment #2)
> Stack trace from Venkman:
> #0: function PH_handleClickToPlayEvent(aPlugin=function:) in
> <chrome://browser/content/browser.js> line 10710
> #1: function anonymous() in <chrome://browser/content/browser.js> line 10608

I account for confirmation.

I confirm with Venkman that aPlugin seems like undefined.
I'm sorry if I could not come across to you via this stack trace.
https://hg.mozilla.org/mozilla-central/rev/f77082549e0e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(In reply to OHZEKI Tetsuharu [:saneyuki_s] from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0
> Firefox/15.0a1
> Build ID: 20120505030510
> 
> Steps to reproduce:
> 
> [Environment]
> http://hg.mozilla.org/mozilla-central/rev/0a48e6561534
> 
> 1. enable click to play
> 2. visit this page:
> http://www.slideshare.net/haricot/how-browser-engines-work/
> 3. Flip a slide to 60.
> 
> 
> Actual results:
> 
> The page loads an embedded YouTube video with flash plugin.
> But browser does not showing click-to-play doorhanger icon.

Now the CtP icon is displayed, but the video won't play after clicking on it. I understand that bug 741130 will fix this ? Tested on Nightly 20-06-2012.
CTP icon is displayed and the video plays after clicking on it.
Verified fixed on Nightly 20.0a1 (2012-12-10) Win 7 x64.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.