851006 - URL does not load when opening private browsing window

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P2)

Description

[Daniela Petrovici]

Attachment: Screenshot with how it looks like when the error reprodces

This is a follow up on bug 827752. The issue is that the URL does not load after opening the PB window. It happens in testFlashCookie.js and testAboutCache.js . The issue is intermittent. 

testAboutCache.js does not fail because we are just waiting for the pages to load, but do not check which were loaded. 

testFlashCookie.js fails because it cannot find an element on the page it did not load. Test results are (with --show-errors):
TEST-START | /home/danielapetrovici/Documents/827752/03_05_2013/mozmill-tests/tests/functional/testPrivateBrowsing/testFlashCookie.js | setupModule
TEST-PASS | /home/danielapetrovici/Documents/827752/03_05_2013/mozmill-tests/tests/functional/testPrivateBrowsing/testFlashCookie.js | testFlashCookie.js::setupModule
TEST-START | /home/danielapetrovici/Documents/827752/03_05_2013/mozmill-tests/tests/functional/testPrivateBrowsing/testFlashCookie.js | testCheckFlashCookie
ERROR | Test Failure: {"exception": {"stack": "MozMillController.prototype.type@resource://mozmill/modules/controller.js:418\ntestCheckFlashCookie@resource://mozmill/modules/frame.js -> file:///home/danielapetrovici/Documents/827752/03_05_2013/mozmill-tests/tests/functional/testPrivateBrowsing/testFlashCookie.js:39\nRunner.prototype.wrapper@resource://mozmill/modules/frame.js:595\nRunner.prototype._runTestModule@resource://mozmill/modules/frame.js:665\nRunner.prototype.runTestModule@resource://mozmill/modules/frame.js:711\nRunner.prototype.runTestFile@resource://mozmill/modules/frame.js:544\nrunTestFile@resource://mozmill/modules/frame.js:723\nBridge.prototype._execFunction@resource://jsbridge/modules/server.js:179\nBridge.prototype.execFunction@resource://jsbridge/modules/server.js:183\n@resource://jsbridge/modules/server.js:283\n", "message": "could not find element ID: cookieValue", "fileName": "resource://mozmill/modules/controller.js", "name": "Error", "lineNumber": 418}}
TEST-UNEXPECTED-FAIL | /home/danielapetrovici/Documents/827752/03_05_2013/mozmill-tests/tests/functional/testPrivateBrowsing/testFlashCookie.js | testFlashCookie.js::testCheckFlashCookie
TEST-START | /home/danielapetrovici/Documents/827752/03_05_2013/mozmill-tests/tests/functional/testPrivateBrowsing/testFlashCookie.js | teardownModule
TEST-PASS | /home/danielapetrovici/Documents/827752/03_05_2013/mozmill-tests/tests/functional/testPrivateBrowsing/testFlashCookie.js | testFlashCookie.js::teardownModule
NOTE: child process received `Goodbye', closing down
INFO Passed: 2
INFO Failed: 1
INFO Skipped: 0

The error for this test is: "could not find element ID: cookieValue"

NOTE: I have also added a screenshot with how it looks when the error appears

Comment 1

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

Even we can't see this problem in the aboutCache test we might end-up in not handling this test correctly given that the page is not cached at all. I consider this important so lets get to it by next week.

Comment 2

[Josh Matthews [:jdm]]

You may be seeing bug 847991, depending on how you trigger the loads.

Comment 3

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

Thanks Josh. Sounds like we hit this. We usually trigger the pb window via a keyboard shortcut or the main menu. Once the window is open and is loaded, we directly fire the request to load another page. And that doesn't seem to work. Daniela, can you please confirm that in such a case e.g. the flash test, you do not see the flash page but about:privatebrowsing?

Comment 4

[Daniela Petrovici]

Yes, this is the case in both testFlashCookie.js and testAboutCache.js

Comment 5

[Daniela Petrovici]

Attachment: patch v1.0 to fix testAboutCache.js

This is the patch to fix the testAboutCache.js test before bug 847991 is fixed.

Comment 6

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

Simple patch to fix this bug for now. I don't see a need to account a sprint entry for.

Comment 7

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

Comment on attachment 724941 [details] [diff] [review]
patch v1.0 to fix testAboutCache.js

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

Instead of modifying each of those test files, why can't we add this to the private browsing ui module?

Comment 8

[Daniela Petrovici]

Attachment: patch v1.1

Modified the private-browsing library per review

Comment 9

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 725304 [details] [diff] [review]
patch v1.1

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/78dcfe10921a (default)

Please check for Aurora as well.

Comment 10

[Daniela Petrovici]

The issue with testAboutCache.js reproduces on Aurora, too. The patch applies cleanly on this branch also

Comment 11

[Andreea Matei [:AndreeaMatei]]

Transplanted:
http://hg.mozilla.org/qa/mozmill-tests/rev/00006aaabfc4 (aurora)

Leaving bug open giving that this is just a workaround.

Comment 12

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

So, what's left on this bug? Can it be closed?

Comment 13

[Daniela Petrovici]

(In reply to Henrik Skupin (:whimboo) from comment #12)
> So, what's left on this bug? Can it be closed?
I don't think it should be closed since the dependent bug is still not fixed. We would need to remove the workaround when bug 847991 is fixed.

Comment 15

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

The landed patch will break the upcoming Mozmill 2.0 testrun given that waitForPageLoad() is called twice. Daniela, please update the manifest to account that.

Comment 16

[Daniela Petrovici]

Attachment: follow-up patch v1.0

(In reply to Henrik Skupin (:whimboo) from comment #15)
> The landed patch will break the upcoming Mozmill 2.0 testrun given that
> waitForPageLoad() is called twice. Daniela, please update the manifest to
> account that.

Actually this is a thing I have forgotten when implementing the change in the open method in pbWindow. 

The open method had been changed so that it will wait for the page to load so that we do not have it added in the tests. But I forgot to check the existing tests where this method was already added. 

I have removed it now from the remaining test. Reports are below:
MAC: mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c21cc7f4
Linux: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c21b7654
Windows: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c21d078c

Comment 17

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

Comment on attachment 749264 [details] [diff] [review]
follow-up patch v1.0

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

So the second waitForPageLoad() call in the test itself is not necessary? Means it will still work with Mozmill 1.5? Why do you comment it out instead of removing it?

Comment 18

[Daniela Petrovici]

Attachment: follow-up patch v1.1

> So the second waitForPageLoad() call in the test itself is not necessary?
> Means it will still work with Mozmill 1.5?
Yes, it will still work in Mozmill 1.5 without it. It was not necessary.

> Why do you comment it out instead of removing it?
Sorry about that. I have attached the correct patch now.

Comment 19

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 749277 [details] [diff] [review]
follow-up patch v1.1

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

http://hg.mozilla.org/qa/mozmill-tests/rev/235e25d5118d (default)

It will need backport for all except esr17.

Comment 20

[Daniela Petrovici]

Attachment: follow up patch v1.0 for Beta and Release

The patch for default applies cleanly on Aurora. Reports are below:
- Windows: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c23455ce
- MAC: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c2343d5f
- Linux: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c231dc67

The patch for default does not apply cleanly on Beta due to the fact that the const TEST_DATA is called TEST_URL. The patch for Beta applies cleanly on Release.

Beta runs:
- Linux: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c23457f7
- MAC: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c234e818
- Windows: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c234f3a9

Release:
- Linux: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c234dd77
- MAC: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c235b164
- Windows: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c235aad8

Comment 21

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 749841 [details] [diff] [review]
follow up patch v1.0 for Beta and Release

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

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/f0f82ed507d6 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/08b5794a2bff (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/590db92913fd (release)

Now we should fix bug 872484 for 2.0.

Comment 22

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

Andreea, can we close this bug as resolved fixed now?

Comment 23

[Andreea Matei [:AndreeaMatei]]

We could close it for now as we will receive a notification email when the dependency is solved (bug 847991).