Closed Bug 717868 Opened 13 years ago Closed 3 years ago

[SeaMonkey] mochitest-plain-4: "test_reftests_with_caret.html | finished in a non-clean fashion" (in /tests/layout/base/tests/test_preserve3d_sorting_hit_testing.html)

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME
mozilla12

People

(Reporter: sgautherie, Unassigned)

References

(Depends on 2 open bugs, Blocks 1 open bug, )

Details

(Keywords: regression, Whiteboard: [qaw: comment 9] [perma-orange])

Attachments

(2 files, 2 obsolete files)

Linux and OSX are affected, not Windows. http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326308477.1326309400.28744.gz OS X 10.5 comm-central-trunk debug test mochitests-4/5 on 2012/01/11 11:01:17 comm-central/59b240e1e76c, mozilla-central/e79ef0ffcb09 No bug. http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326358621.1326360358.31888.gz Linux comm-central-trunk debug test mochitests-4/5 on 2012/01/12 00:57:01 comm-central/49f1b07d5c78, mozilla-central/8ffdb4c7404a { TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_reftests_with_caret.html | application timed out after 330 seconds with no output } http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326354350.1326355395.22602.gz OS X 10.5 comm-central-trunk debug test mochitests-4/5 on 2012/01/11 23:45:50 comm-central/49f1b07d5c78, mozilla-central/8ffdb4c7404a http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326355935.1326356833.25154.gz OS X 10.6 comm-central-trunk debug test mochitests-4/5 on 2012/01/12 00:12:15 comm-central/49f1b07d5c78, mozilla-central/8ffdb4c7404a { 1783 INFO TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_reftests_with_caret.html | finished in a non-clean fashion (in /tests/layout/base/tests/test_preserve3d_sorting_hit_testing.html) } Regressed. Fwiw, http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/test_preserve3d_sorting_hit_testing.html?force=1 Regression timeframes: http://hg.mozilla.org/comm-central/pushloghtml?fromchange=59b240e1e76c&tochange=49f1b07d5c78 (Assuming nothing related here...) http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e79ef0ffcb09&tochange=8ffdb4c7404a (Lots of changesets there...)
(In reply to Serge Gautherie (:sgautherie) from comment #0) > http://hg.mozilla.org/mozilla-central/ > pushloghtml?fromchange=e79ef0ffcb09&tochange=8ffdb4c7404a Bug 668728 caused/exposed this! And that bug was explicitly filed with test_reftests_with_caret as an example ;-<
Blocks: 668728
Summary: [SeaMonkey] mochitest-plain-4: test_reftests_with_caret.html fails since 2012/01/11 → [SeaMonkey] mochitest-plain-4: "test_reftests_with_caret.html | finished in a non-clean fashion"
Summary: [SeaMonkey] mochitest-plain-4: "test_reftests_with_caret.html | finished in a non-clean fashion" → [SeaMonkey] mochitest-plain-4: "test_reftests_with_caret.html | finished in a non-clean fashion" (in /tests/layout/base/tests/test_preserve3d_sorting_hit_testing.html)
Assignee: nobody → sgautherie.bz
Blocks: 512295, 585922
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla12
(In reply to Ehsan Akhgari [:ehsan] from comment #2) > I don't know enough > about SM to tell why, but nothing in those two tests seems suspicious... I can't reproduce locally with an opt build, but, hopefully, there is nothing SM specific here, except SM tinderboxes being slower (I assume).
Attachment #588473 - Flags: review?(ehsan)
Comment on attachment 588473 [details] [diff] [review] (Av1) test_reftests_with_caret.html: Fix "SimpleTest.finish()" handling [Checked in: Comment 8] Does this patch actually fix the problem? I see nothing in the log which indicates that this test is timing out...
(In reply to Serge Gautherie (:sgautherie) from comment #3) > I can't reproduce locally with an opt build, (In reply to Ehsan Akhgari [:ehsan] from comment #4) > Does this patch actually fix the problem? I see nothing in the log which > indicates that this test is timing out... There is no mean to know that in advance, unless manually applying the patch on a (production) SM tinderbox :-| (Or someone else being able to reproduce locally (with a debug build).) Nonetheless, except for adding SimpleTest.executeSoon() which is probably "not required" (but good to have), these changes are wanted to fix/improve this very test at least.
(In reply to Serge Gautherie (:sgautherie) from comment #0) > TEST-UNEXPECTED-FAIL | > /tests/layout/base/tests/test_reftests_with_caret.html | application timed > out after 330 seconds with no output (In reply to Ehsan Akhgari [:ehsan] from comment #4) > I see nothing in the log which indicates that this test is timing out... Don't you?
Comment on attachment 588473 [details] [diff] [review] (Av1) test_reftests_with_caret.html: Fix "SimpleTest.finish()" handling [Checked in: Comment 8] Well, we need to figure out why the test page navigates back, but if you wanna give this a shot, it's fine by me.
Attachment #588473 - Flags: review?(ehsan) → review+
Comment on attachment 588473 [details] [diff] [review] (Av1) test_reftests_with_caret.html: Fix "SimpleTest.finish()" handling [Checked in: Comment 8] https://hg.mozilla.org/mozilla-central/rev/3eaa7d9f1c69 (In reply to Ehsan Akhgari [:ehsan] from comment #7) > Well, we need to figure out why the test page navigates back Agreed, as a next step.
Attachment #588473 - Attachment description: (Av1) test_reftests_with_caret.html: Fix "SimpleTest.finish()" handling → (Av1) test_reftests_with_caret.html: Fix "SimpleTest.finish()" handling [Checked in: Comment 8]
(In reply to Serge Gautherie (:sgautherie) from comment #0) > http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326358621.1326360358. > 31888.gz > Linux comm-central-trunk debug test mochitests-4/5 on 2012/01/12 00:57:01 > comm-central/49f1b07d5c78, mozilla-central/8ffdb4c7404a > { > TEST-UNEXPECTED-FAIL | > /tests/layout/base/tests/test_reftests_with_caret.html | application timed > out after 330 seconds with no output > } This build (and at least 2 other builds) was on "s: cn-sea-qm-centos5-01". But, Linux sometimes times out in the next test after this one instead: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326502474.1326503275.27024.gz Linux comm-central-trunk debug test mochitests-4/5 on 2012/01/13 16:54:34 (before patch A) http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326550634.1326551813.7581.gz Linux comm-central-trunk debug test mochitests-4/5 on 2012/01/14 06:17:14 (after patch A) { s: cb-seamonkey-linux-02 TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_scroll_selection_into_view.html | application timed out after 330 seconds with no output } Not very helpful, but this seems to confirm a timing dependency (in some previous test) :-| (In reply to Serge Gautherie (:sgautherie) from comment #5) > (Or someone else being able to reproduce locally (with a debug build).) That would really help: qawanted. > (In reply to Ehsan Akhgari [:ehsan] from comment #7) > > Well, we need to figure out why the test page navigates back I already had a first glance at your comment 2 analysis but I didn't notice anything obvious to me in that code. (In reply to Serge Gautherie (:sgautherie) from comment #8) > https://hg.mozilla.org/mozilla-central/rev/3eaa7d9f1c69 As it was most likely, this fix(!) did not help with this very bug.
Keywords: qawanted
Whiteboard: [perma-orange] → [qaw: comment 9] [perma-orange]
(In reply to Serge Gautherie (:sgautherie) from comment #9) > (In reply to Serge Gautherie (:sgautherie) from comment #0) > > { > > TEST-UNEXPECTED-FAIL | > > /tests/layout/base/tests/test_reftests_with_caret.html | application timed > > out after 330 seconds with no output > > } > > This build (and at least 2 other builds) was on "s: cn-sea-qm-centos5-01". > > { > s: cb-seamonkey-linux-02 > TEST-UNEXPECTED-FAIL | > /tests/layout/base/tests/test_scroll_selection_into_view.html | application > timed out after 330 seconds with no output > } http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326405904.1326408216.27007.gz Linux comm-central-trunk debug test mochitests-4/5 on 2012/01/12 14:05:04 { s: cb-seamonkey-linux-02 TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_reftests_with_caret.html | application timed out after 330 seconds with no output } Same error but with the other box (before patch A): truly random.
Depends on: 718205
Blocks: 389321
This looks like the likely cause of what you hinted at in comment 2 ;-) Should fix: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326554558.1326555482.19367.gz&fulltext=1 OS X 10.6 comm-central-trunk debug test mochitests-4/5 on 2012/01/14 07:22:38 { 1753 INFO TEST-PASS | /tests/layout/base/tests/test_reftests_with_caret.html | Reftest http://mochi.test:8888/tests/layout/base/tests/bug240933-2.html ... JavaScript error: , line 0: uncaught exception: Unknown key: VK_VK_BACK_SPACE ... 1754 INFO TEST-PASS | /tests/layout/base/tests/test_reftests_with_caret.html | Reftest http://mochi.test:8888/tests/layout/base/tests/bug389321-1.html }
Attachment #588663 - Flags: review?(ehsan)
Priority: -- → P2
Comment on attachment 588663 [details] [diff] [review] (Bv1) bug389321-1.html: Fix "Unknown key: VK_VK_BACK_SPACE" [Checked in: Comment 13] Argh! Good catch. :-) Can you please file a folow-up bug to make sendKey a bit smarter here so that it won't choke when you pass something prefixed with "VK_" to it? Thanks!
Attachment #588663 - Flags: review?(ehsan) → review+
Comment on attachment 588663 [details] [diff] [review] (Bv1) bug389321-1.html: Fix "Unknown key: VK_VK_BACK_SPACE" [Checked in: Comment 13] https://hg.mozilla.org/mozilla-central/rev/1a2426b19376
Attachment #588663 - Attachment description: (Bv1) bug389321-1.html: Use "BACK_SPACE", not "VK_BACK_SPACE" → (Bv1) bug389321-1.html: Fix "Unknown key: VK_VK_BACK_SPACE" [Checked in: Comment 13]
Depends on: 718487
Hopefully, this bug will also fix all intermittent Firefox failures: https://bugzilla.mozilla.org/buglist.cgi?list_id=2076330&short_desc=test_reftests_with_caret.html&query_format=advanced&short_desc_type=casesubstring "32 bugs found." ***** (In reply to Ehsan Akhgari [:ehsan] from bug 717868 comment #12) > Can you please file a folow-up bug to make sendKey a bit smarter Bug 718487.
Comment on attachment 588971 [details] [diff] [review] (Cv1) bug389321-1.html and browser_keyevents_during_autoscrolling.js: (correctly) Use EventUtils sendChar() and sendKey() [Moved to bug 718545 and bug 718546] >diff --git a/layout/base/tests/bug389321-1.html b/layout/base/tests/bug389321-1.html >--- a/layout/base/tests/bug389321-1.html >+++ b/layout/base/tests/bug389321-1.html >@@ -1,18 +1,19 @@ > <!DOCTYPE HTML><html><head> > <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script> > </head> > <body> > <span contenteditable id="t" style="border: 1px dashed green; min-height: 2px; padding-right: 20px;"> </span></body> > <script> >- // Enter a character in the span and delete it > var sel = window.getSelection(); > sel.removeAllRanges(); > >+ // Focus the span to put the caret at its beginning. > var area = document.getElementById('t'); > area.focus(); > >- sendKey("W"); // enter a character >+ // Enter a character in the span then delete it. >+ sendChar("W"); There are not equivalent. Why are you changing this? >diff --git a/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js b/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js This test is not relevant to this bug, is it?
Attachment #588971 - Flags: review?(ehsan) → review-
Please let's not turn this bug into a random place for all kinds of patches and fixes. We should figure out the SM perma-orange in this bug, and nothing else. :-)
No longer blocks: 389321
(In reply to Ehsan Akhgari [:ehsan] from comment #16) > >- sendKey("W"); // enter a character > >+ sendChar("W"); > > There are not equivalent. Why are you changing this? Hence my patch: I am changing this per the very comment on its use and send*() documentations. If using sendKey() is actually required, it should be documented (better). > >diff --git a/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js b/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js > > This test is not relevant to this bug, is it? It seemed easier to me to fix all send*() issue/nits here... (In reply to Ehsan Akhgari [:ehsan] from comment #17) > Please let's not turn this bug into a random place for all kinds of patches > and fixes. We should figure out the SM perma-orange in this bug, and > nothing else. :-) I filed bug 718545 and bug 718546.
(In reply to Serge Gautherie (:sgautherie) from comment #18) > (In reply to Ehsan Akhgari [:ehsan] from comment #16) > > > >- sendKey("W"); // enter a character > > >+ sendChar("W"); > > > > There are not equivalent. Why are you changing this? > > Hence my patch: I am changing this per the very comment on its use and > send*() documentations. > If using sendKey() is actually required, it should be documented (better). Does that change fix this bug?
Attachment #588971 - Attachment description: (Cv1) bug389321-1.html and browser_keyevents_during_autoscrolling.js: (correctly) Use EventUtils sendChar() and sendKey() → (Cv1) bug389321-1.html and browser_keyevents_during_autoscrolling.js: (correctly) Use EventUtils sendChar() and sendKey() [Moved to bug 718545 and bug 718546]
Attachment #588971 - Attachment is obsolete: true
Depends on: 718868
Depends on: 718873
(In reply to Serge Gautherie (:sgautherie) from comment #0) > Linux and OSX are affected, not Windows. (In reply to Serge Gautherie (:sgautherie) from comment #3) > I can't reproduce locally with an opt build, Stupid me, I am on Windows 2000 ;-< ***** (In reply to Serge Gautherie (:sgautherie) from comment #11) > (Bv1) bug389321-1.html: Fix "Unknown key: VK_VK_BACK_SPACE" > > This looks like the likely cause of what you hinted at in comment 2 ;-) I was being hopeful, though not convinced: exception fixed, but not this bug :-| ***** (In reply to Ehsan Akhgari [:ehsan] from comment #19) > (In reply to Serge Gautherie (:sgautherie) from comment #18) > > (In reply to Ehsan Akhgari [:ehsan] from comment #16) > > > > > >- sendKey("W"); // enter a character > > > >+ sendChar("W"); > > Does that change fix this bug? I just checked bug 718546 in: we'll see, but I don't have real hope wrt that "nit". ***** (In reply to Ehsan Akhgari [:ehsan] from comment #2) > So, something either in > <http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/bug682712-1. > html?force=1> or > <http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/bug682712-1- > ref.html?force=1> causes the browser to go back to the previous test > <http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/ > test_preserve3d_sorting_hit_testing.html?force=1>. I don't know enough > about SM to tell why, but nothing in those two tests seems suspicious... Eventually looking deeper into this 3rd hint: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326804272.1326805142.11349.gz&fulltext=1 OS X 10.5 comm-central-trunk debug test mochitests-4/5 on 2012/01/17 04:44:32 test_preserve3d_sorting_hit_testing.html executes correctly: { 1747 INFO TEST-START | /tests/layout/base/tests/test_preserve3d_sorting_hit_testing.html ... (TEST-PASS on its 5 tests) 1753 INFO TEST-END | /tests/layout/base/tests/test_preserve3d_sorting_hit_testing.html | finished in 695ms } Then test_reftests_with_caret.html correctly executes all (but last of) its sub-tests: { 1754 INFO TEST-START | /tests/layout/base/tests/test_reftests_with_caret.html ... 1786 INFO TEST-PASS | /tests/layout/base/tests/test_reftests_with_caret.html | Reftest http://mochi.test:8888/tests/layout/base/tests/bug664087-2.html } Then its 'bug682712-1*.html' turn: Code is { 138 if (!isWindows) { ... 142 tests.push([ 'bug664087-2.html' , 'bug664087-2-ref.html' ]); // bug 680578 143 tests.push([ 'bug682712-1.html' , 'bug682712-1-ref.html' ]); // disabled on Windows 144 } } which explains why Windows is not affected ;-> Log reports: { Firefox: 1 JS strict warning. I filed bug 718873. (Ftr, this sub-test succeeds nonetheless.) SeaMonkey: 1 (different) JS strict warning. I filed bug 718868. SM also reports 1 "WARNING: failed to load URL", which I just logged in that bug (too) ftb. At this point, the latter seems the more likely cause of the current bug... } Then test_preserve3d_sorting_hit_testing.html is implicitly executed again, and this bug (failure) is reported ;-)
Yes, that is precisely why I was suggesting that something is triggering a back navigation.
FWIW, something like that was happening which caused us to notice this problem in the first place.
(In reply to Ehsan Akhgari [:ehsan] from comment #21) > Yes, that is precisely why I was suggesting that something is triggering a > back navigation. Yeah, I was just confirming with more details. (In reply to Ehsan Akhgari [:ehsan] from comment #22) > FWIW, something like that was happening which caused us to notice this > problem in the first place. Interesting. Yet bug 682712 simply added a one-liner to shared 'editor/libeditor/base/nsEditor.cpp', which doesn't give much clue about the SeaMonkey side :-| Or are you referring to something else?
(In reply to Serge Gautherie (:sgautherie) from comment #20) > I just checked bug 718546 in: we'll see, but I don't have real hope wrt that > "nit". http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326855452.1326856383.5763.gz Linux comm-central-trunk debug test mochitests-4/5 on 2012/01/17 18:57:32 As expected, bug 718546 didn't help here.
(In reply to Serge Gautherie (:sgautherie) from comment #23) > (In reply to Ehsan Akhgari [:ehsan] from comment #22) > > FWIW, something like that was happening which caused us to notice this > > problem in the first place. > > Interesting. > Yet bug 682712 simply added a one-liner to shared > 'editor/libeditor/base/nsEditor.cpp', > which doesn't give much clue about the SeaMonkey side :-| > Or are you referring to something else? I'm talking about a different bug, but bugzilla being semi-down doesn't really help me a lot. :/ Really somebody should just set a breakpoint on the docshell Back methods (don't remember exactly what they're called) and see what triggers the back navigation. I think we have exhausted all of the code inspection routes for figuring this out.
Blocks: 682712
(In reply to Ehsan Akhgari [:ehsan] from comment #25) > > Yet bug 682712 simply added a one-liner to shared > > I think we have exhausted all of the code inspection routes for > figuring this out. Looking closer at bug 682712 and its test, I am puzzled by its code: collapse(, 0): I think the current '1' is wrong (as in copy+paste typo). Windows: after changing the value, this test passes on my local (as all the other "non-Windows" tests already do). You may not want to enable this one yet, though? Submitted as https://hg.mozilla.org/try/rev/86a32644ef13 ... NB: If I understood this code correctly, then the question would be why does this test succeeds currently on Linux and MacOSX... If I'm wrong, please explain and let's document better what this code is trying to do.
Attachment #590435 - Flags: review?(ehsan)
Comment on attachment 590435 [details] [diff] [review] (Cv1) bug682712-1.html: Fix and document this test, Enable it on Windows [Moved to bug 720626] Can you please move this to another bug?
Attachment #590435 - Flags: review?(ehsan)
(In reply to Serge Gautherie (:sgautherie) from comment #26) > Created attachment 590435 [details] [diff] [review] > (Cv1) bug682712-1.html: Fix and document this test, Enable it on Windows > > (In reply to Ehsan Akhgari [:ehsan] from comment #25) > > > Yet bug 682712 simply added a one-liner to shared > > > > I think we have exhausted all of the code inspection routes for > > figuring this out. > > Looking closer at bug 682712 and its test, I am puzzled by its code: > > collapse(, 0): I think the current '1' is wrong (as in copy+paste typo). Hmm yeah, that definitely looks wrong.
No longer blocks: 682712
(In reply to Ehsan Akhgari [:ehsan] from comment #27) > Comment on attachment 590435 [details] [diff] [review] > (Cv1) bug682712-1.html: Fix and document this test, Enable it on Windows > > Can you please move this to another bug? Bug 720626.
Attachment #590435 - Attachment description: (Cv1) bug682712-1.html: Fix and document this test, Enable it on Windows → (Cv1) bug682712-1.html: Fix and document this test, Enable it on Windows [Moved to bug 720626]
Attachment #590435 - Attachment is obsolete: true
Fwiw, Still no clue: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1330542720.1330543840.18227.gz Linux comm-central-trunk debug test mochitests-4/5 on 2012/02/29 11:12:00 { TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_reftests_with_caret.html | application timed out after 330 seconds with no output } Added ((ir)relevant) warnings since: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1330652293.1330654046.1058.gz Linux comm-central-trunk debug test mochitests-4/5 on 2012/03/01 17:38:13 { WARNING: GetDefaultCharsetForLocale: need to add multi locale support: file /builds/slave/comm-cen-trunk-lnx-dbg/build/mozilla/intl/locale/src/unix/nsUNIXCharset.cpp, line 150 WARNING: TokensToQueries(), ignoring unknown key: : file /builds/slave/comm-cen-trunk-lnx-dbg/build/mozilla/toolkit/components/places/nsNavHistoryQuery.cpp, line 897 WARNING: excludeItemIfParentHasAnnotation: file /builds/slave/comm-cen-trunk-lnx-dbg/build/mozilla/toolkit/components/places/nsNavHistoryQuery.cpp, line 898 TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_reftests_with_caret.html | application timed out after 330 seconds with no output }
Moving to p3 because no activity for at least 1 year(s). See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3
Moving to p3 because no activity for at least 1 year(s). See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Keywords: qawanted
QA Whiteboard: qa-not-actionable

The bug assignee didn't login in Bugzilla in the last 7 months.
:dholbert, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: bugzillamozillaorg_serge_20140323 → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(dholbert)

Given that there have been no reports of issues for 10 years, and given that this testcase (test_reftests_with_caret.html ) isn't disabled or skipped on any core platforms[1], I'm going to assume this issue went away at some point.

[1] it's only annotated as skipped on TSAN, presumably due to those builds being slower.

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(dholbert)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: