Closed Bug 1022279 Opened 5 years ago Closed 5 years ago

NetworkGeolocationProvider should honor wifi.enabled setting changes

Categories

(Core :: DOM: Geolocation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
blocking-b2g 1.4+
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: dougt, Assigned: dougt)

References

Details

Attachments

(2 files)

On FirefoxOS, it would be nice if the NetworkGeolocationProvider could know when the wifi settings changed so that could switch over to just reporting CELL changes.
Attached patch bug_1022279Splinter Review
jdm, can you review.

Garvan, I think this is what you were thinking.
Attachment #8436431 - Flags: review?(josh)
Blocks: 1022283
Comment on attachment 8436431 [details] [diff] [review]
bug_1022279

Review of attachment 8436431 [details] [diff] [review]:
-----------------------------------------------------------------

steal.

I notice that we used result.value in settingsCallback and now use result as boolean directly. After looking at SettingsService.js, I think it is correct to use result as value directly.
Attachment #8436431 - Flags: review?(josh) → review+
Use cases: 
1) Wifi is on, App using watch position and wifi gets shut off.
2) Wifi is off, App using watch position and wifi gets turned on.

Is this case handled? I expected to see the settings callback address this.
https://hg.mozilla.org/mozilla-central/rev/819fc3d1f260
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
blocking-b2g: --- → 1.4?
Can you please help understand how this impacts the user?
Flags: needinfo?(dougt)
without the 4 changesets I nominated, html5 geolocation will behavior incorrectly.  developers will get location information from potentially 4 location sources each with different accuracies.  These changes improve this and provide a nice filter so that web applications do not have to worry about the implementation details.  I would expect without these changes, most geolocation applications will not work correctly.
Flags: needinfo?(dougt)
OS: Linux → All
Hardware: x86_64 → All
Can we check on 1.3?
Flags: needinfo?(dougt)
Preeti,
What are you asking?
Flags: needinfo?(praghunath)
preeti ping?
Flags: needinfo?(dougt)
Let me try asking the question I think that was trying to being asked here.

Doug - Why is this specifically needed for 1.4? This isn't a regression, right? comment 0 hints that this is nice to have, so triage is confused why this is needed.
Flags: needinfo?(praghunath) → needinfo?(dougt)
Doug,

Sorry for my delayed response.

I was trying to ask if this issue is a regression? What is the priority of the geolocation feature in 1.4? I know it impacts Dolphin so I will nominate this for 1.4D instead.
Thanks Jason 

> Why is this specifically needed for 1.4?

read the comment 7

> This isn't a regression, right?

Not is a serious bug in the geolocation stack.  We patch it for 1.3T, but did not fix it for dolphin.

> comment 0 hints that this is nice to have, so triage is confused why this is needed.

I see.  Here is the rub:

Approve this and the other 3 bugs, or geolocation is going to suck on this platform.
Flags: needinfo?(dougt)
blocking-b2g: 1.4? → 1.4+
Actually wait on that blocking flag - we're planning on taking this to 1.4D. So we don't need this on 1.4 right now.
blocking-b2g: 1.4+ → 1.4?
On 6/23 a decision was made to not provide Dolphin a separate branch but retain 1.4 as ground for Dolphin bugs. Hence making it 1.4+
blocking-b2g: 1.4? → 1.4+
Doug: Ryan says you need to rebase this patch for 1.4.
Flags: needinfo?(dougt)
There is bug in the 1.4 version of this patch, a line got dropped:

This line http://mxr.mozilla.org/mozilla-central/source/dom/system/NetworkGeolocationProvider.js#153
is missing from 1.4:
http://mxr.mozilla.org/mozilla-b2g30_v1_4/source/dom/system/NetworkGeolocationProvider.js#163
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
status-b2g1.4=affected as per comment 19
Garvan: can you please post a new 1.4 patch to add the missing line for wifi.enabled setting? dougt can't because he is on PTO this week.
Flags: needinfo?(gkeeley)
Summary: NetworkGeolocationProvider should honor wifi.enable setting changes → NetworkGeolocationProvider should honor wifi.enabled setting changes
Patch for 1.4 to add the missing line.
Flags: needinfo?(gkeeley)
Attachment #8449442 - Flags: review?(cpeterson)
Comment on attachment 8449442 [details] [diff] [review]
0001-bug1022279-1.4-patch-missing-this-line.patch

Review of attachment 8449442 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8449442 - Flags: review?(cpeterson) → review+
checkin-needed for mozilla-b2g30_v1_4
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/04dd47e806fe

Also, please don't reopen bugs for issues with branch uplifts. That's what status flags are for. Bug resolution tracks the status on trunk.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.