Closed
Bug 994093
Opened 10 years ago
Closed 10 years ago
Provide toggle for Geolocation service (geo.enabled) in Privacy & Security preference pane
Categories
(SeaMonkey :: Preferences, defect)
SeaMonkey
Preferences
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.30
People
(Reporter: rsx11m.pub, Assigned: rsx11m.pub)
References
Details
Attachments
(2 files, 2 obsolete files)
41.34 KB,
image/png
|
Details | |
6.63 KB,
patch
|
rsx11m.pub
:
review+
neil
:
ui-review+
|
Details | Diff | Splinter Review |
While working on the about:rights update (bug 884098) I've noticed the awkward wording of the Location Aware Browsing disable instructions in the Firefox version. I know that this is an opt-in service (subject to bug 903439 to make it work again) but an easier way to disable it completely would be desirable. For the main Privacy & Security pane, I'd envision something like: --- User Tracking --- ... --- Location Aware Browsing --- Websites may request to obtain more information about your current location. (*) Prompt me for permission if a website makes a request ( ) Disable this feature and deny all requests --- Safe Browsing --- ... I'd prefer the 2-radiobutton version over a single checkbox as it allows to better convey what the options are doing (where the main point of annoyance is the doorhanger showing up when you don't really want to use that service). Using "more" is on purpose given that the IP address will be available anyway.
Tentatively taking myself, opinions welcome.
Assignee: nobody → rsx11m.pub
Comment 2•10 years ago
|
||
> Tentatively taking myself, opinions welcome.
I say go for it!
Here we go. The patch is fairly straight-forward and just defines the suggested additional group for geo.enabled with the description slightly shortened to fit into a single line. While I'm there, I'm also cleaning up the attribute alignment in the DNT prefs introduced with bug 835134 and remove the redundant orientation attribute from that radiogroup. The more interesting question is how to deal with bug 903439, given that this preference is a no-op right now. My suggestion here is to disable the group for release builds (assuming that #ifdef RELEASE_BUILD works in SeaMonkey the same way as it does for Firefox and Thunderbird where it's used for this purpose), thus it's available for the testers (and especially the strings are in already). Once the API issue is solved, it takes a simple follow-up patch to make it visible in release builds as well (I'll post that patch in bug 903439 once this gets approved with the exclusion). Adding Help content for the Geolocation feature is bug 494424, and bug 998787 is aiming for a change in the doorhanger notification, thus I'm not adding anything here. I'll attend that bug once the preferences/notification UI is finalized.
Attachment #8409654 -
Flags: ui-review?(neil)
Attachment #8409654 -
Flags: review?(iann_bugzilla)
Now, if I only had attached the correct patch... :-[
Attachment #8409654 -
Attachment is obsolete: true
Attachment #8409654 -
Flags: ui-review?(neil)
Attachment #8409654 -
Flags: review?(iann_bugzilla)
Attachment #8409655 -
Flags: ui-review?(neil)
Attachment #8409655 -
Flags: review?(iann_bugzilla)
Comment 6•10 years ago
|
||
(Why does this need an #ifdef?)
So that the UI is visible in builds of all channels but release until the Geo API is back. Is there a better way to determine which channel we are on while waiting for bug 903439? I don't want to overdo it and introduce some scripting for this, the preprocessor handling is what Thunderbird and Firefox are doing in such cases and can be easily reverted in a follow-up.
Something like if(Services.prefs.getDefaultBranch("").getCharPref("app.update.channel") == "release") document.getElementById("geoLocationGroup").hidden = true; should do the same on the script side, if you'd prefer that over the preprocessing.
Comment 9•10 years ago
|
||
(In reply to rsx11m from comment #7) > So that the UI is visible in builds of all channels but release until the > Geo API is back. > Is there a better way to determine which channel we are on while waiting for > bug 903439? Well, that's three months away, you never know, bug 903439 might have been resolved by then... if it hasn't, perhaps we can back out the content part of this patch on the appropriate repo, if Callek's OK with that. (In reply to rsx11m from comment #8) > Something like > > if(Services.prefs.getDefaultBranch("").getCharPref("app.update.channel") > == "release") > document.getElementById("geoLocationGroup").hidden = true; > > should do the same on the script side, if you'd prefer that over the > preprocessing. Actually it wouldn't work for distro packages, because they use the default channel.
Flags: needinfo?(bugspam.Callek)
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #9) > Well, that's three months away, you never know, bug 903439 might have been > resolved by then... Sure, would be nice, but then - that issue is pending since August 2013 now, thus my thinking.
Comment 11•10 years ago
|
||
Comment on attachment 8409655 [details] [diff] [review] Patch with correct #ifndef >+<!ENTITY geoLocation.label "Location Aware Browsing"> >+<!ENTITY geoIntro.label "Websites may request more information about your current location."> >+ >+<!ENTITY geoEnabled.label "Prompt me for permission if a website makes a request"> Perhaps "Prompt me for permission if a request is made"? >+<!ENTITY geoEnabled.accesskey "m"> >+<!ENTITY geoDisabled.label "Disable this feature and deny all requests"> >+<!ENTITY geoDisabled.accesskey "D"> Yet to test.
Assignee | ||
Comment 12•10 years ago
|
||
(In reply to Ian Neal from comment #11) > Perhaps "Prompt me for permission if a request is made"? Makes sense, given that the description makes the "website requests" context of the label unambiguous.
Comment 13•10 years ago
|
||
Comment on attachment 8409655 [details] [diff] [review] Patch with correct #ifndef r=me with that wording change
Attachment #8409655 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 14•10 years ago
|
||
Per today's discussion with mcsmurf and Ratty over IRC in the status meeting, it appears that a solution to the Geolocation API key isn't coming any time soon. Thus, I'll stick with the #ifndef solution unless Callek tells me otherwise, and we can back it out again at the time when such a key becomes available. Carrying forward r=IanN from attachment 8409655 [details] [diff] [review] with string changed.
Attachment #8409655 -
Attachment is obsolete: true
Attachment #8409655 -
Flags: ui-review?(neil)
Attachment #8414447 -
Flags: ui-review?(neil)
Attachment #8414447 -
Flags: review+
Assignee | ||
Comment 15•10 years ago
|
||
Neil: Ping for ui-review, based on his silence it doesn't seem that Callek has any strong preference.
Assignee | ||
Comment 16•10 years ago
|
||
Neil, Callek: Any suggestion how to proceed here?
Flags: needinfo?(neil)
Comment 17•10 years ago
|
||
Comment on attachment 8414447 [details] [diff] [review] Patch with updated label Sorry for dropping the ball here.
Attachment #8414447 -
Flags: ui-review?(neil) → ui-review+
Assignee | ||
Comment 18•10 years ago
|
||
Thanks, cancelling the needinfo requests and adding this to the check-in queue then.
Comment 19•10 years ago
|
||
Bah, my comment didn't auto-cancel the needinfo :-(
Comment 20•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/8613ef2656da
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.30
You need to log in
before you can comment on or make changes to this bug.
Description
•