Last Comment Bug 678420 - Test for bug 613433 causes browser to navigate BACK unexpected, losing test results
: Test for bug 613433 causes browser to navigate BACK unexpected, losing test r...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Selection (show other bugs)
: Trunk
: All Windows 7
: -- normal (vote)
: mozilla9
Assigned To: Jonathan Griffin (:jgriffin)
:
Mentors:
: 669274 (view as bug list)
Depends on: 680164
Blocks: 677964
  Show dependency treegraph
 
Reported: 2011-08-11 18:13 PDT by Jonathan Griffin (:jgriffin)
Modified: 2011-08-28 13:31 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
mochitest patch (2.10 KB, patch)
2011-08-15 12:51 PDT, Jonathan Griffin (:jgriffin)
ehsan: review+
Details | Diff | Review

Description Jonathan Griffin (:jgriffin) 2011-08-11 18:13:14 PDT
The test for bug 613433, part of /tests/layout/base/tests/test_reftests_with_caret.html, uses this code:

        sendKey("X", window);
        sendKey("BACK_SPACE", window);

This backspace is being caught by the browser, resulting in a BACK navigation, which loads the previous test in the current test's context.  This causes all the test results for the current test to be lost, and in fact has caused some test failures to be hidden in TBPL.  See bug 677964.

If you apply some setTimeouts to the test, you can also see that the "X" is not appearing in the contenteditable div this test is targeting.
Comment 1 Jonathan Griffin (:jgriffin) 2011-08-11 18:13:39 PDT
Ehsan, since this is your test, can you take a look at it when you have a chance?
Comment 2 :Ehsan Akhgari (out sick) 2011-08-15 11:49:54 PDT
(In reply to comment #1)
> Ehsan, since this is your test, can you take a look at it when you have a
> chance?

Yes, I know what's happening.  The backspace key is being captured by the chrome event handler and it's causing a back navigation event.  I think this happens because the event is not properly targeted at the contentEditable div.   Can you try giving an ID to the div and then passing that ID to sendKey to make it dispatch the event to the div directly?
Comment 3 Jonathan Griffin (:jgriffin) 2011-08-15 12:51:10 PDT
Created attachment 553245 [details] [diff] [review]
mochitest patch

Thanks Ehsan, this patch fixes the problem as you suggested.
Comment 4 Jonathan Griffin (:jgriffin) 2011-08-17 10:54:44 PDT
Pushed as http://hg.mozilla.org/mozilla-central/rev/700eb783676a
Comment 5 Phil Ringnalda (:philor) 2011-08-18 00:11:34 PDT
Backed out as http://hg.mozilla.org/mozilla-central/rev/8b970cb862f2

Confusing as all get out, but:

Luke's push Tuesday morning had failures in two reftests_with_caret, bug240933-1.html and bug612271-2.html, on Win7, which I misstarred as being the timeout bug, bug 665408.

The next two pushes had no Win M4 failures.

Your push of this got no Win7 opt tests, because it took so many tries to do the build that the tests got coalesced upward. It had two Win7 debug runs, because for some unfathomable reason I called the first one an upload timeout and retriggered it, maybe from looking before the log was finished uploading, since now it's clearly a failure of five reftests_with_caret. The retriggered run, I finally realized that a failure is not a timeout, and filed bug 679924 on a pair of reftest_with_caret failures.

The next push was for bug 679320, touching editor, so when it failed on Win7 opt, failed on WinXP opt, and failed twice on Win7 debug, I backed it out.

Then my backout failed on Win7 opt and Win7 debug, and I came looking at you ;)
Comment 6 Jonathan Griffin (:jgriffin) 2011-08-18 10:33:30 PDT
discussed this with philor; what really happened is that this fix revealed some long-standing failures in this test case which were previously hidden (for as long as 9 months); will have to follow up on those individual errors separately.
Comment 7 Jonathan Griffin (:jgriffin) 2011-08-18 10:50:22 PDT
*** Bug 669274 has been marked as a duplicate of this bug. ***
Comment 8 Jonathan Griffin (:jgriffin) 2011-08-22 11:38:36 PDT
Landed again as http://hg.mozilla.org/mozilla-central/rev/1720b28e3115
Comment 9 Marco Bonardo [::mak] 2011-08-23 02:09:58 PDT
I backed out this from inbound (will be merged to central once verified it helps), since the increase of M4 failures on Windows was exponential, we have not seen a single green M4 from this push neither on central or inbound (25 oranges out of 25 changesets).

I understand these are real failures and must not be funny to deal with them, but the most frequent failures should be fixed before pushing this again.  This should at least show some green M4 on try before hitting central.
Comment 10 Marco Bonardo [::mak] 2011-08-23 08:57:47 PDT
the backout has been merged to central
http://hg.mozilla.org/mozilla-central/rev/4b8e0357f28c
Comment 11 Phil Ringnalda (:philor) 2011-08-24 11:59:23 PDT
OK, does http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=861ee717f60b look green enough?
Comment 12 Jonathan Griffin (:jgriffin) 2011-08-24 12:13:26 PDT
(In reply to Phil Ringnalda (:philor) from comment #11)
> OK, does http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=861ee717f60b
> look green enough?

Looks great!
Comment 13 Phil Ringnalda (:philor) 2011-08-27 18:11:21 PDT
Once more unto the breach, http://hg.mozilla.org/integration/mozilla-inbound/rev/6385d30812f3
Comment 14 Ed Morley [:emorley] 2011-08-28 13:31:42 PDT
http://hg.mozilla.org/mozilla-central/rev/6385d30812f3

Note You need to log in before you can comment on or make changes to this bug.