503714 - [SeaMonkey 2.1] mochitest-chrome: test_cleanup_search.xul, test_clear_button_disabled.xul and test_delete_key_removes.xul fail

Status: RESOLVED WORKSFORME

(SeaMonkey :: Download & File Handling, defect)

Description

[Serge Gautherie (:sgautherie)]

Tests pass on SMv2.0 (3 platforms), but:

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090711 SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/47bfcd275ede
 +http://hg.mozilla.org/comm-central/rev/291cbe3374b9)

{
6675 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/suite/common/downloads/tests/test_cleanup_search.xul | Clear list is enabled for default 2 item view - got true, expected false

6684 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/suite/common/downloads/tests/test_clear_button_disabled.xul | The clear list button is  not disabled initially.
}

*****

Nit: Remove one space between "is  not".

Comment 1

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090715 SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/d190d9b6ccd1
 +http://hg.mozilla.org/comm-central/rev/773809454cf2)

Additional failure, after bug 474622:
{
6723 INFO Running chrome://mochikit/content/chrome/suite/common/downloads/tests/test_delete_key_removes.xul...
6724 INFO TEST-PASS | ... | The database and the number of downloads display matches
6725 ERROR TEST-UNEXPECTED-FAIL | ... | The download was properly removed - got 6, expected 5
6726 ERROR TEST-UNEXPECTED-FAIL | ... | The download was properly removed - got 6, expected 4
6727 ERROR TEST-UNEXPECTED-FAIL | ... | The download was properly removed - got 6, expected 3
6728 ERROR TEST-UNEXPECTED-FAIL | ... | The download was properly removed - got 6, expected 2
6729 ERROR TEST-UNEXPECTED-FAIL | ... | The download was properly removed - got 6, expected 1
6730 ERROR TEST-UNEXPECTED-FAIL | ... | The download was properly removed - got 6, expected 0
}

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> Additional failure, after bug 474622:

Ftr, the 6 SM 2.1 "TEST-UNEXPECTED-FAIL" were "TEST-KNOWN-FAIL" and are now SM 2.0 "TEST-PASS".

Comment 3

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)

http://mxr.mozilla.org/comm-central/source/suite/common/downloads/tests/chrome/test_cleanup_search.xul
http://mxr.mozilla.org/comm-central/source/suite/common/downloads/tests/chrome/test_clear_button_disabled.xul

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090715
SeaMonkey/2.0b1pre] (experimental/_m-c_, home, optim default) (W2Ksp4)

(http://hg.mozilla.org/mozilla-central/rev/1057ca8f2a91
 +http://hg.mozilla.org/comm-central/rev/d1a9b79cf5c2)

Passes.

(http://hg.mozilla.org/mozilla-central/rev/cabb8925dcd3
 +http://hg.mozilla.org/comm-central/rev/d1a9b79cf5c2)

Fails.

Regression timeframe:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1057ca8f2a91&tochange=cabb8925dcd3
2 changesets:
bug 488846 is Firefox only,
then this is a "Bug 178324, refactor focus" fallout.


(In reply to comment #1)

http://mxr.mozilla.org/comm-central/source/suite/common/downloads/tests/chrome/test_delete_key_removes.xul

Maybe this has the same cause!?

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

I guess we'll have to port bug 498805 (apply it to all of our dlmgr tests). If I'm right this could be done in this bug.

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> I guess we'll have to port bug 498805 (apply it to all of our dlmgr tests).

I tried win.focus(); which did not make any difference.
I tried win.document.focus(); which stops the test(s) and I can see that the button is actually disabled.

I think SeaMonkey rather misses updateClearListButton() from bug 430486.

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #5)
> (In reply to comment #4)
> > I guess we'll have to port bug 498805 (apply it to all of our dlmgr tests).
> 
> I tried win.focus(); which did not make any difference.

True -> probably not needed at all for now.

> I think SeaMonkey rather misses updateClearListButton() from bug 430486.

I don't think we need updateClearListButton(), our isCommandEnabled() works fine... if it is invoked (with m-c it isn't, see below).

I agree though that this is a regression from focus refactoring bug 178324: If you add window.focus(); before each window.updateCommands("tree-select"); line in treeView.js (especially the one inside initTree) all three failing tests pass (the window of test_clear_button_disabled.xul doesn't close but I couldn't find a way to make it do that so let's handle that as a different problem for the moment).

I guess (!) what broke this was that part (bug 178324 comment 126):
"- update command dispatcher code, it was setting the focus on the current
   window instead of the root window"

Obviously I have a m-c build environment now but still I don't feel like I'm qualified to judge what the correct fix is. KaiRo, you originally ported the Download Manager, can you? If not maybe you know who to CC. Maybe Neil?

Comment 7

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a1pre) Gecko/20100123 SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)

This bug seems to have been fixed in the meantime :-)
I'll just reread it before resolving it...

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

sgautherie: ping. Where could I see whether this still happens or not? Or are these tests deactivated (if, how?)?