Closed Bug 885965 Opened 11 years ago Closed 11 years ago

Intermittent browser_typeAheadFind.js | Test timed out | Found a browser window after previous test timed out

Categories

(Toolkit :: Find Toolbar, defect)

Product:
Toolkit
Component:
Find Toolbar
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla25
Tracking Flags:
Tracking Status
firefox24 --- fixed
firefox25 --- fixed

People

(Reporter: ttaubert, Assigned: jaws)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

Patch v2
1.25 KB, patch
ttaubert
: review+
Details | Diff | Splinter Review
Attached patch Patch (obsolete) — — Splinter Review 
My test for bug 882977 made this go perma-orange, but with these changes to browser_typeAheadFind.js this timeout goes away. The previous approach of relying on focus doesn't seem as durable as this approach.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #773087 - Flags: review?(ttaubert)
Comment on attachment 773087 [details] [diff] [review]
Patch

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

::: browser/base/content/test/browser_typeAheadFind.js
@@ +10,5 @@
>    waitForExplicitFinish();
>  
> +  registerCleanupFunction(function() {
> +    Services.prefs.setCharPref("browser.startup.homepage", oldHomepage);
> +    Services.prefs.setIntPref("browser.startup.page", oldPage);

I didn't use Services.prefs.clearUserPref here because browser.startup.page has a value of 1 when this test begins but calling Services.prefs.clearUserPref("browser.startup.page") sets the value to 0.
Blocks: 882977
Comment on attachment 773087 [details] [diff] [review]
Patch

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

I doesn't seem right to tinker with browser.startup.homepage just to make this test work. Locally, replacing 'pageshow' with 'load' makes the test work for me together with the test for bug 882977.
Attachment #773087 - Flags: review?(ttaubert)
Attached patch Patch v2 — — Splinter Review 
I only wish I had thought of that approach first :P

I also removed the use of arguments.callee since it's deprecated.
Attachment #773087 - Attachment is obsolete: true
Attachment #773409 - Flags: review?(ttaubert)
(In reply to Jared Wein [:jaws] from comment #9)
> I also removed the use of arguments.callee since it's deprecated.

Good catch.
Attachment #773409 - Flags: review?(ttaubert) → review+
https://hg.mozilla.org/mozilla-central/rev/a50080f91ba8
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: