869366 - Tap to play plugin setting has no effect

Status: RESOLVED DUPLICATE of bug 866390

(Firefox for Android Graveyard :: Plugins, defect)

Description

[Adrian Tamas (:AdrianT)]

Nightly 23.0a1 2013-05-06
Samsung Galaxy Tab 2 (Android 4.1)/Samsung Galaxy R (Android 2.3.4)/ HTC Desire Z  (Android 2.3.3) / Acer Iconia Tab A500 (Android 3.2) 

Steps to reproduce:
1) Make sure the plugin setting is set to "Tap to play"
2) Go to http://people.mozilla.org/~mwargers/tests/flash/flashembed.html and wait for the page to load

Expected results:
The flash content is not displayed until the user taps in the plugin placeholder to activate it

Actual results:
The plugin is active by default and the content is displayed

Notes:
The "Disable" option works as intended

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Margaret - Can you reproduce this?

Comment 2

[Teodora Vermesan (:TeoVermesan)]

The regression window for this issue is:
1. mozilla central
good build: 24.04.2013 
bad build:  25.04.2013 

pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fef5f202b2dc&tochange=690b5e0f6562

2. inbound
good build: 1366820270
bad build:  1366820510

pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a4fc2d70eade&tochange=450bbfd48532

Comment 3

[:Margaret Leibovic]

Thanks for finding a regression range. It looks like this must have been caused by bug 549697.

Comment 4

[:Margaret Leibovic]

Looks like this was probably caused by this change to nsPluginHost:
http://hg.mozilla.org/integration/mozilla-inbound/diff/450bbfd48532/dom/plugins/base/nsPluginHost.cpp

David, can you advise on how this change may have broken our click to play implementation, and how we can fix it?

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

In moving from blanket click-to-play to per-plugin click-to-play, it became the case that both plugins.click_to_play must be true and the plugin tag's enabledState be nsIPluginTag::STATE_CLICKTOPLAY for a plugin to be click-to-play. On desktop, a plugin tag's enableState can be changed in about:addons -> Plugins, but that doesn't exist on Android.
I think the best way to fix this is to implement a pref that specifies what plugins should default to if they don't otherwise have a pref set for their enabledState. For now, it would be STATE_ENABLED on desktop and STATE_CLICKTOPLAY for Android. This is basically bug 866390 comment 9. We might also need to do a reset on Android if existing plugin tags are set to STATE_ENABLED.

Comment 6

[:Margaret Leibovic]

(In reply to David Keeler (:keeler) from comment #5)
> In moving from blanket click-to-play to per-plugin click-to-play, it became
> the case that both plugins.click_to_play must be true and the plugin tag's
> enabledState be nsIPluginTag::STATE_CLICKTOPLAY for a plugin to be
> click-to-play. On desktop, a plugin tag's enableState can be changed in
> about:addons -> Plugins, but that doesn't exist on Android.
> I think the best way to fix this is to implement a pref that specifies what
> plugins should default to if they don't otherwise have a pref set for their
> enabledState. For now, it would be STATE_ENABLED on desktop and
> STATE_CLICKTOPLAY for Android. This is basically bug 866390 comment 9. We
> might also need to do a reset on Android if existing plugin tags are set to
> STATE_ENABLED.

Are you planning to implement this pref as part of bug 866390 (or another follow-up bug)? If not, do we need to take responsibility for implementing that?

This sounds like it's a core issue, and unfortunately I'm not as familiar with the core implementation as I used to be.

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I'm working on fixing this in bug 866390.

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

What I'm /hoping/ will fix this just landed, so if someone with flash on their phone tests it out when a build is available, we'll know for sure.

Comment 9

[:Margaret Leibovic]

(In reply to David Keeler (:keeler) from comment #8)
> What I'm /hoping/ will fix this just landed, so if someone with flash on
> their phone tests it out when a build is available, we'll know for sure.

I'll pull and make a build now. Thanks so much!

Comment 10

[:Margaret Leibovic]

Yes, this is fixed by bug 866390. Marking as a dupe.