586640 - Timeout failure in waitForPageLoad when a page has already been loaded

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Aaron Train [:aaronmt]]

Attachment: Testcase (testHomeButton.js)

This bug was found when I was changing a test in bug 585775: [1] testPreferences/testSetToCurrentPage.js

By converting the test to use a local-file, the timeout would occur after we press the Home button. The homepage is set to a local-file during the run of a test. 

[1] http://hg.mozilla.org/qa/mozmill-tests/file/8a7873bbfe8b/firefox/testPreferences/testSetToCurrentPage.js

[2] Timeout occurs on this line http://hg.mozilla.org/qa/mozmill-tests/file/8a7873bbfe8b/firefox/testPreferences/testSetToCurrentPage.js#l83

I've attached a simple version of the test. Place it in the testPreferences folder and give it a try.

The test will open a local-file. Open the preferences dialog and set that page to the current page. Open a second local-file. Click the home button. When we click the home button, the test times out with the wait.

Comment 1

[Aaron Train [:aaronmt]]

Tested in 1.4.1: no issues

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 465227 [details]
Testcase (testHomeButton.js)

Can you please attach a testcase without any shared module dependency? thanks.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I would assume its a regression from bug 503277. Before that patch had been landed we were using win.documentLoaded to check when a page has been successfully loaded. By using the property it was passing even when the page has already been loaded and waitForPageLoad has been called too late.

We should still check this property before we start our loop to wait for the DOMContentLoaded event. Using that way we would return early and don't end up in a timeout.

Heather, in my opinion it should block 1.4.2.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v1

The proposed way will fix the problem for all pages except error pages. There is still no reliable way to detect when those have been loaded.

Comment 5

[Heather Arthur [:harth]]

marking blocking since its a regression from a recent feature.

Comment 6

[Heather Arthur [:harth]]

Comment on attachment 465263 [details] [diff] [review]
Patch v1

looks good.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Landed on master and 1.4.2:
http://github.com/mozautomation/mozmill/commit/6c84b6b530607fe3e3ae188bcd7737081bf15264
http://github.com/mozautomation/mozmill/commit/c271e0316dd54c7907018f082c1050a6d0021e5e