858827 - Error callback should be called when timeout is zero and there's no cached position for getCurrentLocation

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Description

[Guilherme Gonçalves [:ggp]]

The spec says setting timeout to zero in getCurrentLocation should have the error callback be called unless there is a cached version with suitable age (that is, smaller than maximumAge) to be used.

In particular, setting timeout to 0 and maximumAge to a high number (potentially Infinity) should result in a cached position be returned if possible, without ever contacting the geolocation provider.

Our implementation, however, sets no timeout in that case, and if there's no cached position to be used, waits until a new position is obtained from the geolocation provider however long that takes.

Comment 1

[nextor]

Attachment: Implementation to see the bug in action

Comment 2

[Alphan Chen [:alchen]]

Attachment: (0122) Add a handling for getCurrentLocation when timeout is zero

Here is a fix for this symptom.

Currently, we don't handle the case when timeout is zero.
However, following the W3C spec, "Error callback" should be called with error code('TIMEOUT') when timeout is zero and there's no cached position for getCurrentLocation.

Comment 3

[Alphan Chen [:alchen]]

For watchPosition, there is no definition in W3C spec.
In my opinion, we can keep the current behavior.

Comment 4

[Josh Matthews [:jdm]]

Comment on attachment 8710817 [details] [diff] [review]
(0122) Add a handling for getCurrentLocation when timeout is zero

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

::: dom/geolocation/nsGeolocation.cpp
@@ +512,5 @@
>      Update(lastPosition.position);
>    } else {
> +    // if it is not a watch request and timeout is 0,
> +    // invoke the errorCallback (if present) with TIMEOUT code
> +    if ((mOptions && (mOptions->mTimeout == 0) > 0) && !mIsWatchPositionRequest) {

I think this can be simplified to `if (mOptions && mOptions->Timeout == 0 && !mIsWatchPositionRequest) {`

Comment 5

[Josh Matthews [:jdm]]

Oh, and a test for this would be nice :)

Comment 6

[Alphan Chen [:alchen]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf05e80a14e0

Comment 7

[Alphan Chen [:alchen]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e7bc2a95ecf

Comment 8

[Alphan Chen [:alchen]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d55b133ce71

Comment 9

[Alphan Chen [:alchen]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e53aae466d4

Comment 10

[Alphan Chen [:alchen]]

Attachment: Add a handling for getCurrentLocation when timeout is zero. r=jdm

The following is the try server result.
It looks fine.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e53aae466d4

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3593d33dd15b

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3593d33dd15b

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3593d33dd15b

Comment 14

[Ben Tian (inactive)]

Set owner to Alphan since he worked on and resolved this bug.