611455 - 'Application disconnected' failure in restartTests/TestMasterPassword/test.2

Status: RESOLVED FIXED

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

Description

[Aaron Train [:aaronmt]]

MODULE: restartTests/testMasterPassword/
TEST: testInvokeMasterPassword
ERROR: Disconnect Error - Application unexpectedly closed 
BRANCH: default
PLATFORM: all

http://hg.mozilla.org/qa/mozmill-tests/file/2fbfb150fefa/firefox/restartTests/testMasterPassword/test1.js#l110

Comment 1

[Aaron Train [:aaronmt]]

Attachment: Dialog

What I'm seeing is the test halting on this dialog (screenshot)

Comment 2

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

When has it been started to fail? The top fail link for this particular test would be more helpful here.

Comment 3

[u279076]

I could not find a top-fail for testInvokeMasterPassword.  However, I did manage to find a top-fail report for the same Disconnect Error in testRemoveMasterPassword:

http://mozmill-release.brasstacks.mozilla.com/#/general/failure?test=firefox%2FrestartTests%2FtestMasterPassword%2Ftest1.js&func=testRemoveMasterPassword

Comment 4

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

Actually we are failing in testRemoveMasterPassword here. Would be good to know in which callback we actually end-up waiting forever. I was able to only reproduce it once locally.

Comment 5

[Aaron Train [:aaronmt]]

This test is disconnecting on the modal dialog that appears in the callback prefDialogDeleteMasterPasswordCallback of  the test testRemoveMasterPassword. It's reproducible for me on Linux on a slower VM. 

The problem is with the DELAY_MODAL_DIALOG set at 500ms and the initial delay before the observer gets called to handle the modal dialog. Setting the timer to test at 1100ms (1.1s) and the test does not halt for me anymore. Dropping that value any more, (e.g, 1000ms) introduced the halt on that dialog again. The default set at 500ms seems to be an issue for slower machines.

Comment 6

[Aaron Train [:aaronmt]]

It's also not restricted to that callback as I just ran it again at 900ms delay and it halted on the checkMasterHandler prompt that requests user input to verify the master password dialog

Comment 7

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

We should really figure out how to fix bug 519714, so we can pass over the current controller. That will help us to compare window.opener with controller.window and to identify if the top most window is the new modal dialog. That way we can remove the sleep calls at all.

But for now it doesn't work and we have to use sleeps. So which values are safe enough? I think even with 1.5s or 2s it wont hurt that much. There is more value in keeping those tests green.

Comment 8

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

I have checked the modal dialog class and it looks like we do not have to wait for bug 519714. We simply have to pass in the controller into the constructor and forward it to the observer. Then we can do the window check. Aaron, please file a bug so we can make this happen.

Comment 9

[Aaron Train [:aaronmt]]

(In reply to comment #8)
> I have checked the modal dialog class and it looks like we do not have to wait
> for bug 519714. We simply have to pass in the controller into the constructor
> and forward it to the observer. Then we can do the window check. Aaron, please
> file a bug so we can make this happen.

bug 613197

Comment 10

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

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.

Comment 11

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

A disconnect shouldn't happen anymore here with bug 613197 fixed.

Comment 12

[Aaron Train [:aaronmt]]

This is reoccurring again with the exact same dialog. I was able to reproduce this with:

Mozmill 1.5.1 and 1.5.2 release candidates, and a recent Minefield nightly, alongside Firefox 4 beta 10

Build identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b11pre) Gecko/20110201 Firefox/4.0b11pre

Comment 13

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

Do we know which of the dialogs it is in detail? We have a lot of modal dialogs in that test.

Comment 14

[Aaron Train [:aaronmt]]

This is occuring in the second test, testInvokeMasterPassword, where we invoke the master password dialog by clicking the ''Show Passwords" button in the Password Manager in the function [1]

[1] http://hg.mozilla.org/qa/mozmill-tests/file/6a13621ad288/firefox/restartTests/testMasterPassword/test1.js#l219

The function does start the new modalDialog object and waits for it after the button click, but the observer doesn't detect it. The password manager window is handled with:

utils.handleWindow("type", "Toolkit:PasswordManager", checkPasswordManager);

The handler works fine [2]

[2] http://hg.mozilla.org/qa/mozmill-tests/file/6a13621ad288/firefox/restartTests/testMasterPassword/test1.js#l237

If I put a sleep of two seconds before the click, the dialog is handled properly, so this is another race condition

Comment 15

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

You should add some dump statements to the findWindow method of the observer. There has to be a reason why it sometimes fail.

I will assign it to you for now, because I'm not available today.

Comment 16

[Aaron Train [:aaronmt]]

Debugging this, it seems like in the findWindow method of the modal-dialog class, both the window and the openers are initialized objects, [object ChromeWindow], but 'found' [1] keeps returning false resulting in a null value returned to the observer. So the following assignment results in false.

[1] found = (mozmill.utils.getChromeWindow(opener) == this._opener);
    
Henrik, what does this mean?

Comment 17

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

(In reply to comment #16)
> [1] found = (mozmill.utils.getChromeWindow(opener) == this._opener);

What's the value of both operands in this case?

Comment 18

[Aaron Train [:aaronmt]]

Is there any way to expose exactly why the opener unwrapped chrome window is not equal to the opener of the modal dialog? There's some kind of race, running this multiple times, sometimes they are equal, sometimes not. 

(In reply to comment #17)
> (In reply to comment #16)
> > [1] found = (mozmill.utils.getChromeWindow(opener) == this._opener);
> 
> What's the value of both operands in this case?

https://developer.mozilla.org/en/nsIDOMChromeWindow, doesn't offer a whole lot to compare against. Where to go from here?

Comment 19

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

You can check:
http://mxr.mozilla.org/mozilla-central/source/embedding/browser/webBrowser/nsIWebBrowserChrome.idl

Take the chrome variable in the find method and get the details like chrome.webBrowser.contentDOMWindow

Comment 20

[Aaron Train [:aaronmt]]

The chrome variable is still wrapped: [xpconnect wrapped nsIWebBrowserChrome], a) should it be wrapped, b) how do I unwrap it?

Comment 21

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

see the line before we check for the opener. There we call the unwrap method of dom-utils. Btw. have you ever did a regression check with older beta releases as b10? I wonder if this could be a Linux only regression. Seems unlikely because it's happening on all branches, but we should check if there was a checkin for Firefox across branches.

Comment 22

[Aaron Train [:aaronmt]]

Mystery here. We start collecting reports (lets say, from 'Disconnect Error' -- see below), starting late January. During that time, we release b10. Running 1.5.2 and builds prior (ran up to b5) still expose this issue, and only on Linux. 

http://mozmill-release.brasstacks.mozilla.com/#/general/failure?branch=4.0&platform=Linux&from=2011-01-01&to=2011-02-11&test=firefox%2FrestartTests%2FtestMasterPassword%2Ftest1.js&func=testInvokeMasterPassword

Comment 23

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

This also happens only on qa-horus but not qa-set. Looks like we have to somehow fix this VM issue.

Comment 24

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

The issue here is that the master password dialog somehow pops-up too early. Not sure if the video output is lagging but meanwhile I believe this problem is caused by the sluggish Ubuntu 10.10 vm. Al will setup new machines we will test. Hopefully those will be fast enough again.

Comment 25

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

I'm waiting for Al who will create a new VM for 10.10 32bit. If an older kernel with better support for VMware tools fixes the problem we will have to do the same for 64bit.  If we can't get it to work we would have to go back to 10.04.

Comment 26

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (default) [disable test2.js][checked-in]

As spoken, we want to disable test2.js in the meantime of current investigation.

Comment 27

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

Comment on attachment 517806 [details] [diff] [review]
Patch v1 - (default) [disable test2.js][checked-in]

r=me. Please get it landed on all branches.

Comment 28

[Aaron Train [:aaronmt]]

Landed across

http://hg.mozilla.org/qa/mozmill-tests/rev/8716df3c8bf6 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/a9a65b8b339a (2.0)
http://hg.mozilla.org/qa/mozmill-tests/rev/f97da75fa38e (1.9.2)
http://hg.mozilla.org/qa/mozmill-tests/rev/4a84485cbede (1.9.1)

Comment 29

[Raul Nicolae Malea]

http://mozmill-crowd.brasstacks.mozilla.com/#/general/report/b0c5cbbd24bf827d6dcfffc745f81fee

Comment 30

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

(In reply to comment #29)
> http://mozmill-crowd.brasstacks.mozilla.com/#/general/report/b0c5cbbd24bf827d6dcfffc745f81fee

That particular test has been disabled and is marked as skipped. It's not a failure but we will have to fix the code to be able to re-enable the test.

Comment 31

[u279076]

Removing assignee until get this reprioritized. Henrik, what needs to be done here?

Comment 32

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

I wasn't able to figure out why we are failing in test2.js. It's an absolutely weird timing issue. Somehow we do not detect the first modal dialog which comes up. I was always able to see this issue on Linux. So if someone could have a quick check if something has been changed since the last patches for the modal dialog handling have been landed, I would kinda appreciate that.

Comment 33

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #32)
> I wasn't able to figure out why we are failing in test2.js. It's an
> absolutely weird timing issue. Somehow we do not detect the first modal
> dialog which comes up. I was always able to see this issue on Linux. So if
> someone could have a quick check if something has been changed since the
> last patches for the modal dialog handling have been landed, I would kinda
> appreciate that.

I was investigating this and reproduced the error manually, the hanging of the exact dialog which Aaron displayed in the screenshot - I have a pretty powerful machine, no VM, Ubuntu 11.04 32b system and Mozmill 1.5.8 

I will move on with the investigation in hope of finding a fix. Assigning myself for the moment

Comment 34

[Maniac Vlad Florin (:vladmaniac)]

My first results of investigation: 

It appear that the 'Password required' window in test2 does not register somewhere in Firefox and it is not accessible, even if we handle the modal dialog correctly. 

Ids are correct, they are no changed, the window just does not exist for Mozmill even if the timer is started with md.start correctly.

Comment 35

[Maniac Vlad Florin (:vladmaniac)]

This is a pastebin of test2.js in which I've dumped and commented what works and what doesn't
http://pastebin.mozilla.org/1477954

Comment 36

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

You should put some dump statements into the waitForDialog() method to figure out if the opened modal dialog is somewhat special and which of our conditions is failing.

Comment 37

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #36)
> You should put some dump statements into the waitForDialog() method to
> figure out if the opened modal dialog is somewhat special and which of our
> conditions is failing.

On it! thanks

Comment 38

[Maniac Vlad Florin (:vladmaniac)]

It seems that we are waiting for the modal dialog and get no response from it which causes a timeout 

Code and log available here http://pastebin.mozilla.org/1478105

Comment 39

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

Please dive in deeper and check the mdObserver's observe method for the real cause.

Comment 40

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #39)
> Please dive in deeper and check the mdObserver's observe method for the real
> cause.

What we do know for sure is that the right window is triggered and focused (the modal dialog) 
but somehow it is not recognized 

http://pastebin.mozilla.org/1478191

Can this be because the modal dialog is triggered by a pref window? 
Focusing on the browser window focuses the modal dialog (controller.window) and focusing the pref window makes the modal dialog to lose focus.

Comment 41

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

So do we pass in the right window for the opener? Looks like we fail here:

found = (mozmill.utils.getChromeWindow(window.opener) == this._opener);

and found is always false because the chrome window doesn't match our opener.

Comment 42

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #41)
> So do we pass in the right window for the opener? Looks like we fail here:
> 
> found = (mozmill.utils.getChromeWindow(window.opener) == this._opener);
> 
> and found is always false because the chrome window doesn't match our opener.

This was left in the queue for some time now, because of other high priority work. Getting back to it, trying a fix

Comment 43

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

We have changed how to wait for the newly set panel in bug 757340. I wonder if this has been changed our situation here. Vlad, is this failure still happening?

Comment 44

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #43)
> We have changed how to wait for the newly set panel in bug 757340. I wonder
> if this has been changed our situation here. Vlad, is this failure still
> happening?

It seems I missed this I will check into it asap

Comment 45

[Maniac Vlad Florin (:vladmaniac)]

Looked into the test and discovered the following: 

* folder testMasterPassword contains the manifest file but not the test, which was renamed to testPreferences_MasterPassword - Remus takes care of this in bug 736764 
* there are a couple of indentation mistakes in the test which were not there in the past afaik 
* also, the error is still reproducible

Comment 46

[Andreea Matei [:AndreeaMatei]]

I had time Friday to investigate a bit and noticed the followings:
- manually reproducing the steps in the browser, the pref window and the modal dialog which requires the master password are "linked" (if clicking the pref window, the modal dialog looses focus, but when focusing the modal dialog, the pref should be in the background, not in the back of the browser as it happens in this test).
- when running the test, we fail at typing the password. If we manually click OK on the modal dialog, without having any password, the dialog will appear again asking for the password (normal behavior). That second time the test manages to type the password, but fails with "Modal dialog has been found and processed" in checkPasswordManager method.
- if we try to manually insert the password, it won't type anything.

I am looking now for older versions of Firefox where the test used to work, at Vlad's suggestion.

Comment 47

[Maniac Vlad Florin (:vladmaniac)]

Thanks Andreea for looking into this. 

What is very strange for me indeed is that you cannot type manually anything in the password modal dialog text field while mozmill test is running. It should work imo

Comment 48

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

Andreea, please see my comment 32 where I have already mentioned that. If you place in a sleep() call before the test is doing anything in the dialog, it will pass. But we don't want to use sleep(), so we have to figure out why we have to wait.

Comment 49

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The issue with typing into Firefox launched via Mozmill is likely to be bug 758233.

Comment 50

[Andreea Matei [:AndreeaMatei]]

Tried to use keypress with AltKey and "P" to access the Password dialog instead of clicking on Saved Passwords button and I noticed this way the dialogs are properly displayed: password dialog, prefs window in the background and then browser window. Is this acceptable for us to use?

Comment 51

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

If that's the case I would consider it a bug in Firefox. Would you mind to give us some simple steps are even better a simplified mozmill testcase we could check? If it's proven we should get a bug on file for Firefox.

Btw. I wonder if that's also the case for inline preferences. Can you please also check this?

Comment 52

[Andreea Matei [:AndreeaMatei]]

Attachment: testcase

I have only replaced 
  controller.click(showPasswordButton);
with
  controller.keypress(null, "P", {altKey: true});
in test2.js - prefDialogInvokeMasterPasswordCallback.
This way the test passes way more than before, but still have intermittent failure to investigate further.

I'm attaching a testcase, but is pretty long since we have to save a password first, to activate and set the master Password in order to access the Password Manager. If a password in not saved first, the Password Manager can be accessed without being asked for the master password.
I have commented two sleeps that I used to see the actual change, otherwise it moves pretty quickly.

Comment 53

[Andreea Matei [:AndreeaMatei]]

Attachment: screenshot

Also attaching a screenshot where you can see the dialogs properly aligned.

Comment 54

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

Hum, why does the masterpassword dialog pops up at all when opening the preferences? Is that the case here? That should really not happen. Please compare with other platforms how those behave. Also I'm still missing feedback regarding inline preferences.

Comment 55

[Andreea Matei [:AndreeaMatei]]

The masterpassword dialog pops up after the keypress that accesses 'Saved Passwords' button. 

Regarding inline preferences, I'm working on it to get to the Security pane, because I can't use prefs module and there are other nodes. For some reason it won't click the button.
After that, I've checked the elements and they are the same.

Comment 56

[Andreea Matei [:AndreeaMatei]]

With inline preferences, keypress won't work, neither if we try manually on the Security pane to press "Alt + P". 
If we click the 'Saved Password' button like it is in test2.js, we have the same issue where it won't type the password.

Since we don't have a window pref but a tab, the problem with dialogs alignment is no longer present here.

Comment 57

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

So I would suggest we go back and check comment 14 and ff. Adding debug statements to the modal dialog class to figure why we do not catch this dialog.

Comment 58

[Daniela Petrovici]

I have added the dumps to both test2.js and modal-dialog.js (will add the patch and the log document in a moment). 

There seems to be an issue I cannot reproduce manually: when clicking on the show passwords button the windows change their position. The most recent window does not have a title (as shown in the attached log file) since it is not the "Password required" one.

As per Andreea's comment #52, changing click on Save Password button fixes the issue intermittently, but the issue also reproduced in checkPasswordManager function with the togglePasswords button.

If we change these two commands to controller.keypress(null, "P", {altKey: true}); the test will pass. I have tried 10 times on Linux and no error appeared if I changed the two instances of clicking on Save Passwords with keypress.

Comment 59

[Daniela Petrovici]

Attachment: patch with dumps

Patch containing dumps

Comment 60

[Daniela Petrovici]

Attachment: log file for the patch with dumps added above

The log file for the patch with dumps.

Test no. 2 starts from FUNCTION: testInvokeMasterPassword.

Please tell me if anything else needs to be tried here in regards to dumps added.

Comment 61

[Daniela Petrovici]

Attachment: patch with restartTests only where test2 is enabled and click changed to keypress

I have run tests with this patch and reports show the test passing 10 out of 10 times. The link to all reports is below (the tests are the ones that have 77 tests totally):
http://mozmill-crowd.blargon7.com/#/functional/reports?branch=21.0&platform=Linux&from=2013-02-07&to=2013-02-07

Comment 62

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The debug confirms comment 41:

IN findWindow - window is: 
IN findWindow - if (window.opener) - false

Compared to running locally against Mac:

IN findWindow - window is: 
IN findWindow - if (window.opener) - true

Could you add dump statements to show window.opener and this._opener as we're expecting them to be the same... It may be that the main window is not the opener, as this modal dialog is presented from within the preferences window. I'm not sure why this would vary between platforms though.

Comment 63

[Daniela Petrovici]

Attachment: patch with dump v1.1

I have added the patch with dumps containing also changes to show the title of the window.opener and this._opener. If I just show window.opener and this._opener it will display: [object ChromeWindow]

I have ran this on Linux and it is passing once out of 5 runs - I will attach the logs for failing and passing runs in a bit.

Comment 64

[Daniela Petrovici]

Attachment: failing run - Linux

This is a failing run and when it fails it dumps the following:
IN findWindow - VALUE OF WINDOW.OPENER - Nightly Start Page - Nightly
IN findWindow - VALUE OF THIS._OPENER - Nightly Preferences
IN findWindow - if (window.opener) - false

Comment 65

[Daniela Petrovici]

Attachment: passing run - Linux

When it passes the dumps show the correct page was found:
IN findWindow - VALUE OF WINDOW.OPENER - Nightly Preferences
IN findWindow - VALUE OF THIS._OPENER - Nightly Preferences
IN findWindow - if (window.opener) - true

Please tell me if I should add anything else here.

Comment 66

[Dave Hunt [:davehunt] [he/him] ⌚BST]

To me, this appears that the handleWindow method in utils is finding the wrong window. Perhaps you could add some debug statements in this method. It's also possible there's an issue in mozmill.utils.getWindowByType.

See: http://hg.mozilla.org/qa/mozmill-tests/file/06ce35a5fa41/lib/utils.js#l333

Comment 67

[Daniela Petrovici]

Attachment: patch v1.2 with dumps

I have added dumps in handleWindow. The test fails in two cases (for which I will add the files with dumps):

1) sometimes the "Password required" is not found after we open Preferences and press the Saved Passwords button
Here we are finding the window through utils.handleWindow:
After assert waitFor in utils.handleWindow: Nightly Preferences
WindowId=14

FUNCTION: prefDialogInvokeMasterPasswordCallback
In test2.js - modalDialog with controller.window (title)=Nightly Preferences
modalDialog - Title of this._window=Nightly Preferences

IN prefDialogInvokeMasterPasswordCallback before md.start(checkMasterHandler)
mdObserver - Title of this._opener=Nightly Preferences

modal-dialog.js - findWindow:
IN findWindow - window is: Password Required
IN findWindow - VALUE OF WINDOW.OPENER - Nightly Start Page - Nightly
IN findWindow - VALUE OF THIS._OPENER (received from utils.handleWindow) - Nightly Preferences

2) sometimes the "Password required" is found after we open Preferences and press the Saved Passwords button, but then we cannot find the Saved Passwords modal dialog because the most recent window is Password Required

After assert waitFor in utils.handleWindow: Saved Passwords
WindowId=16

IN checkPasswordManager
modalDialog - Title of this._window=Saved Passwords
mdObserver - Title of this._opener=Saved Passwords
FUNCTION: md.start() - making sure this._observer is not null: [object Object]
END OF md.start()

IN findWindow - window is: Nightly Preferences
IN findWindow - !chrome.isWindowModal

IN OBSERVER - modal-dialog.js - window = null
IN OBSERVER - modal-dialog.js - TRY AGAIN IN A BIT

IN findWindow - window is: Password Required
IN findWindow - VALUE OF WINDOW.OPENER - Nightly Start Page - Nightly
IN findWindow - VALUE OF THIS._OPENER (received from utils.handleWindow) - Saved Passwords

IN findWindow - if (window.opener) - false
IN OBSERVER - modal-dialog.js - window = null
IN OBSERVER - modal-dialog.js - TRY AGAIN IN A BIT
IN findWindow - window is: Password Required
IN findWindow - VALUE OF WINDOW.OPENER - Nightly Start Page - Nightly

Comment 68

[Daniela Petrovici]

Attachment: dumps for case 2

Comment 69

[Daniela Petrovici]

Attachment: dumps for case 1

Comment 70

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Thanks Daniela, please suggest what we should do next.

Comment 71

[Daniela Petrovici]

I think that the utils.handleWindow returns the correct value for the window because when the test passed, the title of the this._opener and window._opener are both "Nightly Preferences"

So it appears that the window.opener is sometimes not returning the correct opener. Although I am not entirely sure, I think that it is due to the fact that the dialogs change position when we click on the Saved Passwords button and we loose focus.

Also, we sometimes fail to find the window (comment #60):
IN findWindow - window is: 
IN findWindow - if (window.opener) - false

and sometimes we find it, but not the correct window.opener:
IN findWindow - window is: Password Required
IN findWindow - VALUE OF WINDOW.OPENER - Nightly Start Page - Nightly

I think that it might be related to: mozmill.wm.getMostRecentWindow('');

Since this was working properly previously, I would like to try with a build from before this issue failed so I can understand what changed.

Please tell me if this is not the correct way to continue

Comment 72

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

(In reply to Daniela Petrovici from comment #71)
> I think that the utils.handleWindow returns the correct value for the window
> because when the test passed, the title of the this._opener and
> window._opener are both "Nightly Preferences"

Well, our question still is what this method returns in case of a failure. Not sure if it's already part of your answer above but I would be interested in. The logs are quite large so I would love to get a condensed answer.

Comment 73

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

Just one random thought. Please check how the preferences window get opened. Is it in the background or delayed? All that could be the reason why we are getting the main browser window as opener by accident.

Comment 74

[Daniela Petrovici]

(In reply to Henrik Skupin (:whimboo) [away 02/09 - 02/017] from comment #72)
> (In reply to Daniela Petrovici from comment #71)
> > I think that the utils.handleWindow returns the correct value for the window
> > because when the test passed, the title of the this._opener and
> > window._opener are both "Nightly Preferences"
> 
> Well, our question still is what this method returns in case of a failure.
> Not sure if it's already part of your answer above but I would be interested
> in. The logs are quite large so I would love to get a condensed answer.

utils.handleWindow returns "Nightly Preferences" window if the test is failing and also if it is passing:
"After assert waitFor in utils.handleWindow: Nightly Preferences"

Comment 75

[Daniela Petrovici]

Attachment: screenshot with what happens when it fails

(In reply to Henrik Skupin (:whimboo) from comment #73)
> Just one random thought. Please check how the preferences window get opened.
> Is it in the background or delayed? All that could be the reason why we are
> getting the main browser window as opener by accident.

The preferences window gets opened in the background. I have created a screenshot so it is visible how it looks like when it fails

Comment 76

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

So that's the problem then. In such a case getMostRecentWindow('') will return the browser window but not the preferences window. We would have to specify the window type to make it explicit.

But it's more interesting WHY it opens in the background. Do you see this manually? If not please try to make the test slower via sleep calls and see if something changes.

Comment 77

[Daniela Petrovici]

(In reply to Henrik Skupin (:whimboo) from comment #76)
> So that's the problem then. In such a case getMostRecentWindow('') will
> return the browser window but not the preferences window. We would have to
> specify the window type to make it explicit.
> 
> But it's more interesting WHY it opens in the background. Do you see this
> manually? If not please try to make the test slower via sleep calls and see
> if something changes.

It doesn't happen manually, and controller.sleep(100) before clicking on the Show passwords button helped => the password was successfully entered in the "Password required" text field, but then the test failed again because the "Saved Passwords" dialog was not found.

But it seems this issue can be fixed with the changes from bug 803492 - adding this.window.focus() to the controller.js. The test passed 40 times out of 40 with the changed mozmill. Report is here: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf78150166536fa - tested on Linux. Will test on MAC and Windows and update issue.

Comment 78

[Daniela Petrovici]

Verified with 40 runs on Windows and Mac and the issue does not reproduce if we add the change in bug 803492. The reports are:
Windows:
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf78150166c2c66
MAC:
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf78150166c017d

Comment 79

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

Sounds great. So lets get this focus patch included in Mozmill 1.5.21.

Comment 80

[Daniela Petrovici]

Attachment: enabled tests patch v1.0

Patch to enable this test

Comment 81

[Daniela Petrovici]

Attachment: enabled tests patch v1.1

Modified the patch since if we add this patch before backing out the patch for bug 841355, the test will not work - test1 and test3 are skipped in bug 841355.

Comment 82

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 717056 [details] [diff] [review]
enabled tests patch v1.1

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

Just one nit to keep the changes our contributor did.

::: tests/functional/restartTests/testPreferences_masterPassword/test2.js
@@ +92,5 @@
>    var button = new elementslib.Lookup(controller.window.document,
>                                 '/id("commonDialog")/anon({"anonid":"buttons"})/{"dlgtype":"accept"}');
>    controller.click(button);
>  }
>  

Please remove this extra line as well.

Comment 83

[Daniela Petrovici]

Attachment: enabled tests patch v1.2

Removed the empty line at the end of the file

Comment 84

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 717093 [details] [diff] [review]
enabled tests patch v1.2

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

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/01a31a5de741 (default).

I'll land on aurora after the backout from bug 841355.

Comment 85

[Daniela Petrovici]

The patch applies cleanly on all branches and the reports are below:
Aurora:
Linux: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf781501697b532
Windows: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016929820
MAC: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016925384

Beta:
MAC: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf781501696db24
Windows: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d3867b
Linux: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d3bcb2

Release:
MAC: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d3d4d0
Windows: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d3fae1
Linux: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d460be

ESR17:
MAC: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d487a8
Windows: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d4c168
Linux: http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016d4bb16

Comment 86

[Andreea Matei [:AndreeaMatei]]

Transplanted as:
http://hg.mozilla.org/qa/mozmill-tests/rev/bb1c181af73b (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/8f1512300e4a (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/7cf750da0ef9 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/d6a1e6489c60 (esr17)

Thanks! Fingers crossed! :)