Closed Bug 1022279 Opened 5 years ago Closed 5 years ago

NetworkGeolocationProvider should honor wifi.enabled setting changes


(Core :: DOM: Geolocation, defect)

Not set



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


(Reporter: dougt, Assigned: dougt)




(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]

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


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.
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)
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)

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
is missing from 1.4:
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]

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

Attachment #8449442 - Flags: review?(cpeterson) → review+
checkin-needed for mozilla-b2g30_v1_4
Keywords: checkin-needed

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.
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.