Closed Bug 1008941 Opened 10 years ago Closed 10 years ago

Add test to verify dismissing the geolocation sharing hanger

Categories

(Core :: DOM: Geolocation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: mihaelav, Assigned: mihaelav, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lang=js])

Attachments

(1 file, 1 obsolete file)

Steps:

1. Open a page which uses geolocation
>> The location sharing hanger is displayed

2. Close the hanger without selecting any sharing option
>> The location is not shared
Mihaela, this will become a browser chrome test, right? Andrei is not the appropriate mentor here. Please find the right one.
Whiteboard: [mentor=andrei.eftimie][lang=js]
Attached patch v1 (obsolete) — Splinter Review
Attachment #8446997 - Flags: review?(gkeeley)
Whiteboard: [mentor=garvank][lang=js]
Mentor: gkeeley
Whiteboard: [mentor=garvank][lang=js] → [lang=js]
Comment on attachment 8446997 [details] [diff] [review]
v1

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

This 99% browser chrome testing and 1% geolocation, so I am going to defer to JDM for a review on this.
Attachment #8446997 - Flags: review?(gkeeley) → review?(josh)
Comment on attachment 8446997 [details] [diff] [review]
v1

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

::: dom/tests/browser/browser_bug1008941_dismissGeolocationHanger.js
@@ +36,5 @@
> +
> +function waitForPageLoad(aTab) {
> +  let deferred = Promise.defer();
> +
> +  let timeoutId = setTimeout(() => {

I don't see the value of having a timeout here - the test will time out eventually anyways, and this gives us the possibility of intermittent oranges if the test machine stalls for some reason (such as a long GC). It's not that likely for the 20s timeout, but the 5s one below is definitely a possibility.

::: dom/tests/browser/position.html
@@ +16,5 @@
> +      result = document.getElementById("result");
> +
> +      if (navigator.geolocation)
> +        navigator.geolocation.getCurrentPosition(handlePosition, handleError,
> +                                                 {enableHighAccuracy: false, timeout: 30000});

Why these options?
Attachment #8446997 - Flags: review?(josh) → review+
Attached patch v1.1Splinter Review
Updated based on review
Assignee: nobody → mihaela.velimiroviciu
Attachment #8446997 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Comment on attachment 8455989 [details] [diff] [review]
v1.1

Carrying over the r+, no failures in tbpl.
Attachment #8455989 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8cc209a3bd19
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Blocks: 1035848
No longer blocks: 1007559
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: