582618 - Click on elements of error and safe browsing pages directly after the page has been loaded doesn't send event

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

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

With Mozmill 1.4.2 and bug 579943 fixed, our tests which are using waitForPageLoad() for error pages will fail due to bug 503277.

We should check all our tests, most of them are located under testSecurity, and remove that function call. Instead we have to call the waitForElement() function.

Aaron, can you please take care of it? Thanks.

Comment 1

[Aaron Train [:aaronmt]]

Is this also handling warning pages?

For example:

testUnknownIssuer

// Go to a website with an unknown cert issuer
controller.open('https://mur.at');
controller.waitForPageLoad(1000);

Comment 2

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

This is an error page. But warning pages like the safe browsing one should also be affected afair.

Comment 3

[Aaron Train [:aaronmt]]

Attachment: waitForElement changes

Currently have the changes working on the following tests

* firefox/testSecurity/testSSLDisabledErrorPage.js
* firefox/testSecurity/testSecurityNotification.js
* firefox/testSecurity/testUnknownIssuer.js

The two other tests (below), when changed are currently timing out on the assertLoadedUrlEqual lines and I'm not exactly sure why

* firefox/testSecurity/testSafeBrowsingWarningPages.js
* firefox/testSecurity/testSafeBrowsingNotificationBar.js

Comment 4

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

Comment on attachment 460973 [details] [diff] [review]
waitForElement changes

Looks good. Follow that path for the others.

Comment 5

[Aaron Train [:aaronmt]]

(In reply to comment #4)
> Comment on attachment 460973 [details] [diff] [review]
> waitForElement changes
> 
> Looks good. Follow that path for the others.

What shall I do with the two tests listed above that need an additional wait. The waitForPageLoad handled the two pages

* http://www.mozilla.com/firefox/its-an-attack.html
* http://www.mozilla.com/firefox/its-a-trap.html

With a change to waitForElement is not a sufficient wait for page completion.

Comment 6

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

Why you want to wait until the full page has been loaded? It's not necessary. Only wait for the element you want to work with.

Comment 7

[Aaron Train [:aaronmt]]

Attachment: testcase

(In reply to comment #6)
> Why you want to wait until the full page has been loaded? It's not necessary.
> Only wait for the element you want to work with.

Times out on the assertNodeNotExist if I take out the waitForPageLoad

Comment 8

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

My patch on bug 503277 will hopefully fix most of the problems with existing tests for error pages. We would only have to update the waitForPageLoad calls.

Comment 9

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

With my patch on bug 503277 error pages will be still able to use waitForPageLoad. The only issue which is left, is clicking elements inside the safe browsing pages. Somehow it doesn't work right after the page has been loaded. The elements are accessible but firing an event like click, doesn't result in a processing and the final page isn't opened. Adding a 200ms sleep after waitForPageLoad solves this issue. Eventually we have to take this workaround if no other way is capable. Further we can get much better report details with bug 583604 implemented. That way we don't fail by checking a target state but already when synthesizing the event. I hope we will be able to get this into 1.4.2 too.

Comment 10

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

Bug 583604 will not be part of Mozmill 1.4.2. So we should investigate if an additional timeout of 200ms after a page load is sufficient or if we need another more robust way. Not sure right now, if we have an alternative.

Aaron, can you run tests against the safe browsing tests with a sleep call set for page loads which are followed by a click on elements? Here the place for the warning pages test module:

  var ignoreWarningButton = new elementslib.ID(controller.tabs.activeTab, "ignoreWarningButton");

  // Wait for the ignoreButton to be safely loaded on the warning page
+ controller.sleep(200);

If that fixes the problem we could do more investigation and push a temporary fix for both tests.

Comment 11

[Aaron Train [:aaronmt]]

(In reply to comment #10)
> Bug 583604 will not be part of Mozmill 1.4.2. So we should investigate if an
> additional timeout of 200ms after a page load is sufficient or if we need
> another more robust way. Not sure right now, if we have an alternative.
> 
> Aaron, can you run tests against the safe browsing tests with a sleep call set
> for page loads which are followed by a click on elements? Here the place for
> the warning pages test module:
> 
>   var ignoreWarningButton = new elementslib.ID(controller.tabs.activeTab,
> "ignoreWarningButton");
> 
>   // Wait for the ignoreButton to be safely loaded on the warning page
> + controller.sleep(200);
> 
> If that fixes the problem we could do more investigation and push a temporary
> fix for both tests.

* testSafeBrowsingWarningPages - test passes with and without changes, i.e., removing 1000's, adding sleep(200); before we click on the button
* testSafeBrowsingNotificationBar - test fails with changes while timing out on waitForPageLoad, i.e., removing goBack's for open, removing 1000's, adding sleep(200); before we click on the button

Comment 12

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

The waitForPageLoad() method still waits 1000ms so we do not encounter this issue right now. But we should take it into account when fixing bug 590048.

Comment 13

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

All that should be fixed with Mozmill 1.5.1. I will leave it open for now.

Comment 14

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

Looks like it has been fixed. If there is an remaining issue please file it as a new bug.

Comment 15

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

Move of Mozmill Test related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.