477631 - [Linux] Intermittent Chrome test_bug418874.xul failure

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Serge Gautherie (:sgautherie)]

I already filed bug 472302, which was morphed and fixed,
yet Linux SeaMonkey still has this issue:

See bug 472302 comment 20, bug 472302 comment 21 and
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1234134281.1234138622.29877.gz
Linux comm-central dep unit test on 2009/02/08 15:04:41

*** 5302 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | undo correctly enabled when emptyText was not changed through property
*** 5304 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | undo correctly enabled when emptyText explicitly changed through property
}

Comment 1

[Graeme McCutcheon [:graememcc]]

Attachment: Add some more output

Failed again this morning on Firefox trunk.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1235800277.1235805155.24753.gz

We seem to be worse off since bug 472302 - when this fails, we now have two failures rather than one.

Dão, what would you think about adding some extra spew to confirm the textboxes are in the state they're supposed to be in?

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)

Then, Firefox too and trunk too.

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 364651 [details] [diff] [review]
Add some more output

>+      is(t1CanUndo.value, true,
>+         "undo correctly enabled when emptyText was not changed through property");

>+      is(t2CanUndo.value, true,
>+         "undo correctly enabled when emptyText explicitly changed through property");

You could add 't*CanUndo.value' to the text, to be even more explicit.

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 364651 [details] [diff] [review]
Add some more output

>+      ok(true, "t1 value is "+t1.value);

>+      ok(true, "t2 value is "+t2.value);

And spaces around the operator.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 364651 [details] [diff] [review]
Add some more output

>+      ok(true, "t1 value is "+t1.value);

Why not is(t1.value, ..., "t1 value");?

>-      ok(t1CanUndo.value, "undo correctly enabled when emptyText was not changed through property");
>+      is(t1CanUndo.value, true,
>+         "undo correctly enabled when emptyText was not changed through property");

What's the point of this change?

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Failed again:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.1/1236051486.1236057120.27846.gz

Comment 7

[Graeme McCutcheon [:graememcc]]

Attachment: Debug spew v2 (checked in)

Apologies for the delay coming back to this - day job/real life getting in the way.

> What's the point of this change?

Paranoia, mostly. I want to be sure that it is still a timing issue, and want to rule out a failure in the canUndo call, so I want to be sure that it is indeed the case that t*.canUndo.value == false, and not undefined/null.

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 365932 [details] [diff] [review]
Debug spew v2 (checked in)

ok(a, "") is equivalent to is(a, true, "")... at least for true/false/null/undefined.

Comment 9

[Serge Gautherie (:sgautherie)]

Still there:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1236406575.1236413545.9435.gz
Linux mozilla-central unit test on 2009/03/06 22:16:15

Comment 10

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1236413248.1236417995.18404.gz
Linux comm-central dep unit test on 2009/03/07 00:07:28

Comment 11

[Dão Gottwald [:dao]]

Is there any new data in the logs? If not, there's no need to link to more of them.

Comment 12

[Graeme McCutcheon [:graememcc]]

> ok(a, "") is equivalent to is(a, true, "")... at least for
> true/false/null/undefined.

Yes, but I don't want the type conversion...that will give us no more information than we have already -  I want the debug spew to report exactly what canUndo holds at that point, to help narrow down what's failing.

Comment 13

[Dão Gottwald [:dao]]

Oh, so you want to get "got foo, expected bar" back. Because for the actual test, the type conversion happens either way...

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 365932 [details] [diff] [review]
Debug spew v2 (checked in)

http://hg.mozilla.org/mozilla-central/rev/03e0942bcb90

> Is there any new data in the logs? If not, there's no need to link to more of
> them.

Now if you see it fail again, please do comment, as we need the new data :)

Comment 15

[:Gavin Sharp [email: [e-mail]]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1236583027.1236589047.9553.gz&fulltext=1#err0

Comment 16

[Serge Gautherie (:sgautherie)]

(In reply to comment #15)
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1236583027.1236589047.9553.gz&fulltext=1#err0

{
*** 780 INFO Running chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul...
NEXT ERROR *** 781 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | t1 value correct following input - got "", expected "1"
*** 782 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | undo correctly enabled when emptyText was not changed through property - got false, expected true
*** 783 INFO TEST-PASS | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | updated emptyText displayed
*** 784 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | t2 value correct following input - got "", expected "2"
*** 785 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | undo correctly enabled when emptyText explicitly changed through property - got false, expected true
}

Comment 17

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/2d6e148e8122
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/31f0a9854d39

let's see if it sticks.

Comment 18

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1237055635.1237060410.14003.gz
Linux comm-central dep unit test on 2009/03/14 11:33:55

Bug still there.

Comment 19

[Dão Gottwald [:dao]]

What's the ALSA lib conf.c stuff about? I'd consider this fixed until it comes back without other bogus stuff outside of this test.

Comment 20

[Phil Ringnalda (:philor)]

The ALSA spew is just bug 469635 not being hacked-around on the SM box(es). That same spew is actually happening in every run (success or failure) on the Fx boxes, too, it's just getting sent to /dev/null there.

Comment 21

[Serge Gautherie (:sgautherie)]

Again:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1237561715.1237568218.22012.gz
Linux mozilla-1.9.1 unit test on 2009/03/20 08:08:35

Comment 22

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1238088083.1238095807.11266.gz
Linux mozilla-1.9.1 unit test on 2009/03/26 10:21:23

Comment 23

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1238873167.1238878561.18777.gz
Linux comm-central unit test on 2009/04/04 12:26:07

Comment 24

[(not currently active) Ted Mielczarek]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1239029874.1239035833.27679.gz
Linux mozilla-1.9.1 unit test on 2009/04/06 07:57:54

This test is clearly not fixed.

Comment 25

[Graeme McCutcheon [:graememcc]]

Attachment: Avoid focus issues [Checkin: Comment 26]

First chance I've had to look at this again for a while. Sorry, all.

OK, thinking about this again. What triggered 418874 was two or more calls of _updateVisibleText when displaying the emptytext, with only one call to _clearEmptyText. We can actually simulate this without sending keypresses - as the keypresses themselves weren't really relevant, it was the focus()/click() calls that were important.

Simply setting the value of the textbox would work, as changes through the property still end up on the undo stack, and we get the correct sequence of _updateVisible/_clearEmpty calls. With this updated patch, reverting the fix for 418874 causes the last part of the test to fail as expected.

Comment 26

[Dão Gottwald [:dao]]

Would have been interesting to know if that was really a focus issue. I don't see why textbox.focus() would fail if the window had focus. Anyway...

http://hg.mozilla.org/mozilla-central/rev/9a8abc2e3a17
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1f3ebba05fb6

Comment 27

[Legacy TBPL/Treeherder Robot]

jorendorff%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1275635415.1275636132.31248.gz
Linux tracemonkey opt test mochitest-other on 2010/06/04 00:10:15

s: moz2-linux-slave29
7444 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | undo correctly enabled when placeholder was not changed through property - got false, expected true
7445 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug418874.xul | undo correctly enabled when placeholder explicitly changed through property - got false, expected true
PROCESS-CRASH | automation.py | application crashed (minidump found)
Thread 0 (crashed)
PROCESS-CRASH | automation.py | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/places/tests/browser/browser_bookmarksProperties.js | Timed out