Plugin doorhanger appears for unsupported plugin embeds (and hidden embeds that are enabled by default)

VERIFIED FIXED in Firefox 14

Status

()

VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: martijn.martijn, Assigned: Margaret)

Tracking

({testcase})

Trunk
Firefox 14
ARM
Android
testcase
Points:
---

Firefox Tracking Flags

(blocking-fennec1.0 soft)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
Created attachment 613272 [details]
testcase

You need to have the Setting for Plugins set to "Tap to Play" to see this bug.

See testcase, the invisible embeds have type=unknown/unknown, so Fennec can't support that plugin.
When visiting that page however, I get to see the plugin doorhanger that asks me if I would like to play that plugin. That doesn't make sense for plugin content that isn't supported.
blocking-fennec1.0: --- → ?
(Assignee)

Comment 1

7 years ago
Bug 739048 should prevent us from firing PluginClickToPlay events for unsupported content, but it looks like this is happening because we're not actually checking if a PluginClickToPlay event fired before deciding to show the doorhanger, only checking if there are plugins:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1826

This also means that we're probably showing the doorhanger for hidden plugins even when plugins are enabled. Martijn, do you happen to know if a bug was filed for that?
(Reporter)

Updated

7 years ago
Blocks: 744060
(Reporter)

Comment 2

7 years ago
(In reply to Margaret Leibovic [:margaret] from comment #1)
> This also means that we're probably showing the doorhanger for hidden
> plugins even when plugins are enabled. Martijn, do you happen to know if a
> bug was filed for that?

I think this is bug 741134.
(Assignee)

Comment 3

7 years ago
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #2)
> (In reply to Margaret Leibovic [:margaret] from comment #1)
> > This also means that we're probably showing the doorhanger for hidden
> > plugins even when plugins are enabled. Martijn, do you happen to know if a
> > bug was filed for that?
> 
> I think this is bug 741134.

That's about when plugins have been enabled for a specific site - I'm talking about the global setting here. I was able to reproduce this problem by changing plugins to enabled in the settings menu, then visiting http://people.mozilla.com/~mwargers/tests/plugins/flash/flashembed_invisible.html.
Wouldn't hold the release for it, but would take a safe fix
blocking-fennec1.0: ? → soft
(Assignee)

Comment 5

7 years ago
I think we really need to fix this, since this means we're showing this doorhanger even when plugins are enabled by default. What we need to do is make sure a PluginClickToPlay event has fired before deciding to show the doorhanger in the load event.

In bug 730318, we decided to stop keeping track of plugins that we saw through the PluginClickToPlay event, and it would be nice to avoid having to rely on that again. Jared, have you found a way to deal with this problem in your desktop patches?
Yeah, on desktop we no longer use the load event, and do all of the calculations when we receive a PluginClickToPlay event.

This has a negative side effect that the first PluginClickToPlay event would determine if the doorhanger should show or not show. Given that, we decided to always show the doorhanger but in a dismissed state.
(Assignee)

Comment 7

7 years ago
(In reply to Jared Wein [:jaws] from comment #6)
> Yeah, on desktop we no longer use the load event, and do all of the
> calculations when we receive a PluginClickToPlay event.
> 
> This has a negative side effect that the first PluginClickToPlay event would
> determine if the doorhanger should show or not show. Given that, we decided
> to always show the doorhanger but in a dismissed state.

Right now we don't have UI for dismissed doorhangers on mobile, but I think that potentially showing a notification even when there's a placeholder is okay, since the user can now choose to set permissions to never see a doorhanger on a site again.

Also, I'm updating the summary to indicate the full scope of this problem.
Assignee: nobody → margaret.leibovic
Summary: Plugin doorhanger appears for unsupported plugin embeds → Plugin doorhanger appears for unsupported plugin embeds (and hidden embeds that are enabled by default)
(Assignee)

Comment 8

7 years ago
Created attachment 613764 [details] [diff] [review]
patch

This patch gets rid of the load listener like Jared suggested, making the PluginClickToPlay event handler the only place that we decide to show the doorhanger. This means that we'll never accidentally show the doorhanger for non-click-to-play plugins.

The downside of this approach is that we will show a notification whenever there is hidden plugin content on the page, even if there are visible "tap to play" overlays. However, like I mentioned before, I don't think this is a big problem, since users can choose to set permissions to never see the doorhanger again for a site.
Attachment #613764 - Flags: review?(mark.finkle)
Attachment #613764 - Flags: feedback?(jwein)
(Reporter)

Comment 9

7 years ago
(In reply to Margaret Leibovic [:margaret] from comment #3)
> > I think this is bug 741134.
> 
> That's about when plugins have been enabled for a specific site - I'm
> talking about the global setting here. I was able to reproduce this problem

Sorry, you're right. I don't think I filed a bug for that, but I guess you've included that case in this bug now.
Attachment #613764 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 10

7 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9a47a875dc0
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/d9a47a875dc0
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 12

7 years ago
Verified fixe on a 2012-04-14 trunk build on the Samsung Galaxy Nexus.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 13

7 years ago
Also verified that the str in comment 3 are working now.
You need to log in before you can comment on or make changes to this bug.