MODULE: testSafeBrowsingNotificationBar.js TEST: testNotificationBar ERROR: controller.waitForPageLoad(): Timeout waiting for page loaded PLATFORM: All BRANCH: All Fixing this unmasked two problems in the test: a) goBack is untrustworthy, and b) another waitForPageLoad() issue resolved by acting on the activeTab rather than getTab.
Created attachment 498472 [details] [diff] [review] Patch v1 - (default) Removed goBack() for open(), and used activeTab instead of getTab
Attachment #498472 - Flags: review?(hskupin)
Comment on attachment 498472 [details] [diff] [review] Patch v1 - (default) >+++ b/firefox/testSecurity/testSafeBrowsingNotificationBar.js > // Go back to the notification bar >- controller.goBack(); >+ controller.open(badSites[i]); > controller.waitForPageLoad(); goBack() uses the bfcache which doesn't work with waitForPageLoad at the moment. See bug 605784. So removing it is the right approach here. >- controller.waitForPageLoad(controller.tabs.getTab(1)); >+ controller.waitForPageLoad(controller.tabs.activeTab); Why does getTab(1) fail? Do we have more than 2 tabs? Can you please give a more detailed explanation?
Attachment #498472 - Flags: review?(hskupin) → feedback+
Created attachment 498738 [details] [diff] [review] Patch v1.1 - (default) When I ran this it seemed to hang on getTab instead of activeTab, although, with some comparison testing now, I'm unable to see the behaviour again at home. I just did a couple runs cross platform and it's not needed. For refactoring, since the tab opened becomes the activeTab, I think it would make sense to change it in the future. Patch for review just has the switch from from goBack() to open()
Attachment #498738 - Flags: review?(hskupin)
Attachment #498738 - Flags: review?(hskupin) → review+
(In reply to comment #3) > home. I just did a couple runs cross platform and it's not needed. For > refactoring, since the tab opened becomes the activeTab, I think it would make > sense to change it in the future. Agreed on. In the same step we should check if we can apply that change also to other existent tests. Just for consistency.
default - http://hg.mozilla.org/qa/mozmill-tests/rev/ef576d0eb075 1.9.2 - http://hg.mozilla.org/qa/mozmill-tests/rev/93eb9734927f 1.9.1 - http://hg.mozilla.org/qa/mozmill-tests/rev/d18792253ec5
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.