Closed Bug 974100 Opened 6 years ago Closed 6 years ago

Don't expose SettingsService in non b2g builds

Categories

(Core :: DOM: Device Interfaces, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox28 --- fixed
firefox29 --- fixed
firefox30 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: gwagner, Assigned: gwagner)

References

Details

(Whiteboard: [systemsfe])

Attachments

(1 file)

Seems like we use SettingsService in desktop land as well. We shouldn't do this.
Attached patch patchSplinter Review
Don't expose SettingsService in non b2g builds.
Assignee: nobody → anygregor
Whiteboard: [systemsfe]
Can we kick off a review here and get this landed? Looks like it'll have to uplift all the way up to beta.

I'm curious what other services that should be available to desktop might be broken due to this, looks like we access this in a few places - 

http://mxr.mozilla.org/mozilla-central/search?string=nsISettingsService
(In reply to Jim Mathies [:jimm] from comment #2)
> Can we kick off a review here and get this landed? Looks like it'll have to
> uplift all the way up to beta.

The reason why I haven't asked for review yet is because this patch also disables all the tests we have for the settingsService. We are not running browser-chrome tests with b2g builds.
Can someone check if this patch fixes the actual geolocation problem on windows?

> 
> I'm curious what other services that should be available to desktop might be
> broken due to this, looks like we access this in a few places - 
> 
> http://mxr.mozilla.org/mozilla-central/search?string=nsISettingsService
I think we need a nightly to confirm that since local builds don't have google api keys (cc'ing dougt).

FWIW, I confirmed that geolocation starts working again if you install a nightly build into a directory that has user write permissions.
Attachment #8377811 - Flags: review?(bent.mozilla)
Attachment #8377811 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/98416c0b888d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Blocks: 970112
Gregor, could you make sure that your change get uplifted to beta and aurora too? thanks
cf bug 970112
Flags: needinfo?(anygregor)
(In reply to Sylvestre Ledru [:sylvestre] from comment #7)
> Gregor, could you make sure that your change get uplifted to beta and aurora
> too? thanks
> cf bug 970112

Sure, but lets be safe and wait a day or two.
Flags: needinfo?(anygregor)
if you need a key, you can use one from https://code.google.com/apis/console/

if you need the mozilla-google key, email me.
Comment on attachment 8377811 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 803451
User impact if declined: geolocation doesn't work
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): 
String or IDL/UUID changes made by this patch:none

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 803451
User impact if declined: geolocation doesn't work
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): 
String or IDL/UUID changes made by this patch: none
Attachment #8377811 - Flags: approval-mozilla-beta?
Attachment #8377811 - Flags: approval-mozilla-aurora?
Attachment #8377811 - Flags: approval-mozilla-beta?
Attachment #8377811 - Flags: approval-mozilla-beta+
Attachment #8377811 - Flags: approval-mozilla-aurora?
Attachment #8377811 - Flags: approval-mozilla-aurora+
Duplicate of this bug: 970112
Flags: in-testsuite?
See Also: → 1012403
Depends on: 1319861
You need to log in before you can comment on or make changes to this bug.