Closed Bug 736278 Opened 8 years ago Closed 8 years ago

Change tap to play behaviour for plugins

Categories

(Firefox for Android :: General, defect, P2)

14 Branch
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 14
Tracking Status
firefox15 --- verified
blocking-fennec1.0 --- beta+
fennec 13+ ---

People

(Reporter: info, Assigned: Margaret)

References

Details

(Keywords: late-l10n, Whiteboard: [landed again (again) on inbound])

Attachments

(2 files)

In Fennec Native, when Plugins are set to "Tap to play" in settings, when one goes to a website with Flash content, the browser comes up with a prompt saying: "The Page contains plugin content. Would you like to play it?" 

This prompt is totally unnecessary and annoying. "Tap to play" implies that the browser should not load the Flash content, unless the user taps on a flash object. The current behaviour forces the user to make a choice every time a page with Flash content is loaded. 

This is also how other browsers on Android (the default browser, as well as Opera and Dolphin HD) treat tap to play. The prompt is unnecessary and is unique to Fennec Native.
There are hidden or small flash elements that would be unclickable this is the cause of this dialog. I don't believe this to be a bug.
I understand your point Kevin, but may I just point out that when browsing a website like economist.com, I have to answer yes/no to this prompt with every page load, which is very annoying. Also no other browser on Android I know of shows a similar prompt. I'm not a UI designer but I would hope that there is a better way of managing this?
Ah. This sounds related to bug 711618, which margaret pointed me to on irc, but sorta the opposite. The idea there was to automatically load plugins if you selected them enough times. I think if the user ever says "No" on a domain, we should probably remember that and never prompt again. Our site settings dialog should be able to handle turning these back on if the user wants? Yanking in madhava.
Status: UNCONFIRMED → NEW
blocking-fennec1.0: --- → ?
Ever confirmed: true
Summary: Change Tab to play behaviour for plugins → Change tap to play behaviour for plugins
Ian do we want to add some sort of "always for this page" or "never for this page" so the user can choose to not have the door hanger show up every time.
Assignee: nobody → ibarlow
Keywords: uiwanted
tracking-fennec: --- → 13+
blocking-fennec1.0: ? → beta+
Keywords: late-l10n
Priority: -- → P2
Any idea on the schedule for getting the work done and landing this?  string freeze depends on it.
We had discussed in triage last week to do what Brad suggested, which is to add an "Always / Never for this website" doorhanger. Does the team need more specific details from UX, or is that enough to go on?
If we're letting people undo it with "Clear Site Settings" do we need an additional string for that dialog? I.e. to go with the checkbox?
(In reply to Ian Barlow (:ibarlow) from comment #6)
> We had discussed in triage last week to do what Brad suggested, which is to
> add an "Always / Never for this website" doorhanger. Does the team need more
> specific details from UX, or is that enough to go on?

Right now we're only showing a doorhanger if there are only hidden plugins on the page, so that "always/never" option would only apply to pages with hidden plugin content. Is that what we want? Otherwise we'd need to show a doorhanger when the user taps a "tap to play" overlay, and that seems more annoying than we'd care to be.

"Never" is also sort of misleading, since if a visible "tap to play" overlay is present, we'd still let a user tap on that to enable the plugins (if not, it's going to be more complicated to try to change the overlay, and I think that's out of scope here).

Also, how granular are these permissions going to be? Per host, eTLD+1?

(In reply to Madhava Enros [:madhava] from comment #7)
> If we're letting people undo it with "Clear Site Settings" do we need an
> additional string for that dialog? I.e. to go with the checkbox?

Yes, we will need a string to go with that. We'll also need string for the never/always options (which can be the same strings we'll use in the doorhanger. (The items in the site settings dialog all require 3 strings like this: http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3990).
This bug is very similar to bug 711618, we should look at that to make sure we don't miss anything (and make sure we follow the same ideas).
1. Yes, we should still only show this doorhanger for hidden plugin content, not for Tap to Play overlays. And if there is both, show the doorhanger.

2. Re: Never being misleading. Ah interesting. So it's not that the user is saying, "I don't ever want to do this again", it's more about "please stop asking me if I want to do this". What about something like below, where the doorhanger still provides a Yes / No choice, but also provides the ability to opt out of future prompts from this site, and it's checked by default.


   +-----------------------------------------------------/\-----+
   |                                                            |
   |   %S contains plugin content. Would you like to play it?   |
   |                                                            |
   |   [x] Don't ask again                                      |
   |                                                            |
   +---------------------------+-------------------------------->
   |                           |                                |
   |           Yes             |           No                   |
   |                           |                                |
   +---------------------------+>-------------------------------+


In which case I think you would only need a Site Setting string if a user had checked the "Don't ask again" box.

String:
Play Plugins
Yes
No
Just wondering for L10n sake, can we use Ok, Cancel instead of Yes/No?  Using Yes/No can be confusing in Japanese. ( http://en.wikipedia.org/wiki/Yes_and_no )
Assignee: ibarlow → margaret.leibovic
Keywords: uiwanted
The checkbox is checked by default, since that's what we want in this bug. We can expand the checkbox options API to also take a checked state if we need it in another bug, but I figured we should keep is as simple as possible for right now.

I tested this with the geolocation doorhanger to make sure regular doorhangers still work right, and it still works correctly (gr, I want some test coverage here).
Attachment #609858 - Flags: review?(mark.finkle)
This was actually pretty simple.
Attachment #609859 - Flags: review?(mark.finkle)
Attachment #609858 - Flags: review?(mark.finkle) → review+
Comment on attachment 609859 [details] [diff] [review]
(2/2) Add click to play always/never permissions


>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

> var PluginHelper = {
>   showDoorHanger: function(aTab) {
>-    let message = Strings.browser.GetStringFromName("clickToPlayPlugins.message");
>+    let uri = aTab.browser.currentURI;

There might be a slim chance that aTab.browser is null. We have seen it before at odd times. Can you defend against that?

  if (!aTab.browser)
    return;
Attachment #609859 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e5e46023094
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a4ed2b028ad

Although it looks like I need to back-out and re-land because a dependent patch needs to be backed out...
These patches were backed out due to test failures on patches beneath these patches that had conflicts with these patches. Please rebase and reland.

https://hg.mozilla.org/integration/mozilla-inbound/rev/15930226f98b
https://hg.mozilla.org/integration/mozilla-inbound/rev/591720c6e345
Backed out again in https://hg.mozilla.org/integration/mozilla-inbound/rev/254382ca03c1 - either this or bug 697309 was busting native talos, hard though it was to see through all the other bustage and the normal bustage. Retriggered on https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=36586538ef3b in between the backout and the relanding, got nothing but green, retriggered on the relanding in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=47afa45afdfb and it was back to red.
Turns out a missing "}" caused the bustage. Fixed now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/cecf156d7869
https://hg.mozilla.org/integration/mozilla-inbound/rev/c86023072076
Target Milestone: --- → Firefox 14
Once again, mfinkle and I pushed at the same time, and talos failed again, so backing out again, this time hopefully we can really find the problem:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8bebfa88961
https://hg.mozilla.org/integration/mozilla-inbound/rev/082d5e045cc2
Whiteboard: [landed again (again) on inbound]
https://hg.mozilla.org/mozilla-central/rev/6d62f902a321
https://hg.mozilla.org/mozilla-central/rev/220fcfedb8a2

\o/

:-)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Depends on: 741134
Comment on attachment 609858 [details] [diff] [review]
(1/2) Add support for checkbox in doorhanger popups

[Approval Request Comment]
Mobile only. Beta blocker.
Attachment #609858 - Flags: approval-mozilla-aurora?
Comment on attachment 609859 [details] [diff] [review]
(2/2) Add click to play always/never permissions

[Approval Request Comment]
Mobile only. Beta blocker.
Attachment #609859 - Flags: approval-mozilla-aurora?
Comment on attachment 609858 [details] [diff] [review]
(1/2) Add support for checkbox in doorhanger popups

beta blocker, mobile only, a=me
Attachment #609858 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 609859 [details] [diff] [review]
(2/2) Add click to play always/never permissions

beta blocker, mobile only, a=me
Attachment #609859 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on:

Firefox 15.0a1 (2012-04-30)
Device: Samsung Captivate
OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.