Closed
Bug 729485
Opened 12 years ago
Closed 12 years ago
Add an "Always Share" option to geolocation doorhanger notifications
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox21 verified, blocking-fennec1.0 -, fennec+)
VERIFIED
FIXED
Firefox 15
People
(Reporter: Ken, Assigned: Margaret)
References
Details
(Whiteboard: [MTD])
Attachments
(3 files)
6.75 KB,
patch
|
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
85.06 KB,
image/png
|
Details | |
5.42 KB,
patch
|
kats
:
review+
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•12 years ago
|
||
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.
Updated•12 years ago
|
Severity: normal → enhancement
blocking-fennec1.0: --- → ?
Assignee | ||
Comment 3•12 years ago
|
||
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
Updated•12 years ago
|
tracking-fennec: --- → +
blocking-fennec1.0: ? → -
Updated•12 years ago
|
Severity: enhancement → normal
Comment 5•12 years ago
|
||
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
Assignee | ||
Comment 6•12 years ago
|
||
(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 | ||
Updated•12 years ago
|
Assignee: nobody → margaret.leibovic
Comment 7•12 years ago
|
||
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.
Assignee | ||
Comment 8•12 years ago
|
||
(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!
Assignee | ||
Comment 9•12 years ago
|
||
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)
Assignee | ||
Comment 10•12 years ago
|
||
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)
Assignee | ||
Comment 11•12 years ago
|
||
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.
Updated•12 years ago
|
Attachment #624125 -
Flags: review?(mbrubeck) → review+
Assignee | ||
Comment 12•12 years ago
|
||
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 13•12 years ago
|
||
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 14•12 years ago
|
||
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+
Assignee | ||
Comment 15•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/62b0be6431ad https://hg.mozilla.org/integration/mozilla-inbound/rev/023c65a66288 Ian, if there are polish issues, we can address them in a follow-up.
Keywords: uiwanted
Target Milestone: --- → Firefox 15
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/62b0be6431ad https://hg.mozilla.org/mozilla-central/rev/023c65a66288
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Assignee | ||
Updated•12 years ago
|
Attachment #624127 -
Flags: feedback?(ibarlow)
Comment 17•11 years ago
|
||
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
status-firefox21:
--- → verified
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•