Closed Bug 729485 Opened 13 years ago Closed 13 years ago

Add an "Always Share" option to geolocation doorhanger notifications

Categories

(Firefox for Android Graveyard :: General, defect)

11 Branch
ARM
Android
defect
Not set
normal

Tracking

(firefox21 verified, blocking-fennec1.0 -, fennec+)

VERIFIED FIXED
Firefox 15
Tracking Status
firefox21 --- verified
blocking-fennec1.0 --- -
fennec + ---

People

(Reporter: Ken, Assigned: Margaret)

References

Details

(Whiteboard: [MTD])

Attachments

(3 files)

Currently on Firefox Aurora for Mobile, a site has to ask for coordinates five times before Firefox stops asking users if it is okay to provide coordinates. This can be really annoying. For instance, on a web app I'm developing for a client we need to ask for coordinates repeatedly because coordinate accuracy tends to improve over time. We need the most accurate coordinates the user's device can provide because they are inspecting storm water control devices (e.g. storm drains, culverts, etc.) and we need to record the location of inspectors when they submit inspections to make sure they were where they thought they were. This app does not have publicly available pages so I can not provide a demo page. What is needed is a site based "yes, don't ask again" option the first time a website asks for coordinates like other mobile browsers have.
Pretty sure this is by design. After five times it becomes permanent.
I do realize this is by design, but it is not intuitive. All other browsers including Firefox on desktops immediately give users an option to not ask again. Doing something different (e.g. prompting five times) is confusing at best and annoying at worst. I think many users, like me, will think something was broken when they get multiple prompts for the same question on the same site.
Keywords: uiwanted
Severity: normal → enhancement
blocking-fennec1.0: --- → ?
Updating the summary to make it easier to read (also we'd probably use a string like "Always Share" if we do this, since that's what we use on desktop).
Summary: When sites ask for coordinates provide a yes, don't ask again option → Add an "Always Share" option to geolocation doorhanger notifications
tracking-fennec: --- → +
blocking-fennec1.0: ? → -
Severity: enhancement → normal
I think it would be best if the various doorhanger notifications would have similar UI. Here you can see all kinds of doorhanger notifications (and their differences) in action: http://people.mozilla.org/~mwargers/tests/dom/multiple_doorhangers.htm
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #5) > I think it would be best if the various doorhanger notifications would have > similar UI. Here you can see all kinds of doorhanger notifications (and > their differences) in action: > http://people.mozilla.org/~mwargers/tests/dom/multiple_doorhangers.htm Ian filed bug 739757 about getting the notifications all in order. We added support for a "always for this site" checkbox in bug 736278, and I think that would be a good UI pattern to follow for geolocation notifications as well. Ian, what do you think? We could nom this to soft-block if you like this idea. It would be a pretty simple change, although I suppose it will involve new strings. Not sure what our current policy on that is.
Assignee: nobody → margaret.leibovic
Margaret, I agree this would be nice to have, though I'm not sure where we stand with adding new strings either. If we can get it in now, great, if not, I've made a note to address this when we tackle bug 739757.
(In reply to Ian Barlow (:ibarlow) from comment #7) > Margaret, I agree this would be nice to have, though I'm not sure where we > stand with adding new strings either. This change isn't a blocker, and it does require strings, so it would be for Firefox 15. We aren't limited for string changes for that, so we can do whatever you want!
Attached patch patchSplinter Review
This patch replaces the "set permanent permission after 5 times" logic with a "Don't ask again for this site" checkbox for permissions notifications that are created by ContentPermissionPrompt.js (right now that's just geolocation and desktop notifications). If this is the direction we want to go for all notifications, we can make sure the other permissions notifications match this in bug 739757 (I already started making a list of notifications for that bug). I also cleaned up some cruft that made its way in here when this file was ported over. I also have a plan to create tests for this! I will work on those today.
Attachment #624125 - Flags: review?(mbrubeck)
Ian, is this what you want? I modeled it after the plugins notification. This patch will also change the desktop notifications notification to look like this except with these strings: http://mxr.mozilla.org/mozilla-central/source/mobile/android/locales/en-US/chrome/browser.properties#91
Attachment #624127 - Flags: feedback?(ibarlow)
Comment on attachment 624125 [details] [diff] [review] patch Review of attachment 624125 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/components/ContentPermissionPrompt.js @@ +97,5 @@ > return; > > + // Bail if there's no window to work with > + if (!request.window) > + return; I was looking at the desktop implementation of this, and it doesn't have this check. It looks like it just existed before in getTabForRequest because of e10s, so I can probably get rid of it here.
Attachment #624125 - Flags: review?(mbrubeck) → review+
Attached patch testsSplinter Review
This only tests the functionality of the "Share" button, both with and without the "Don't ask again for this site" checkbox checked. Unfortunately, it's basically impossible to test for the one-time "Don't Share" functionality, since no callback gets triggered. I plan to test the permanent "Don't Share" functionality, but I need to do something to clear the permission first (this could add test coverage for our "Clear Site Settings" menu option). I figured this start is better than nothing, though! Asking Kats to take a look at this, since it uses colors and PixelTest stuff to communicate the state of geolocation permissions.
Attachment #624243 - Flags: review?(mbrubeck)
Attachment #624243 - Flags: review?(bugmail.mozilla)
Comment on attachment 624243 [details] [diff] [review] tests >+ private PaintedSurface reloadPage() { Maybe we should add this to the parent class; it seems like it could be a useful general utility function.
Attachment #624243 - Flags: review?(mbrubeck) → review+
Comment on attachment 624243 [details] [diff] [review] tests Review of attachment 624243 [details] [diff] [review]: ----------------------------------------------------------------- I agree with Matt, I think reloadPage should be moved up into PixelTest, and leave the delay constant private. Also I've noticed that pixels on the edges tend to have more variability in the GL readback; I would suggest sampling a pixel from 10,10 or such rather than 0,0 just to reduce chances of random oranges.
Attachment #624243 - Flags: review?(bugmail.mozilla) → review+
Keywords: uiwanted
Target Milestone: --- → Firefox 15
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Attachment #624127 - Flags: feedback?(ibarlow)
This option was added on the latest Nightly. Closing bug as verified fixed on: Firefox for Android Version: 21.0a1 (2013-01-29) Device: Galaxy R OS: Android 2.3.4
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: