Closed Bug 569988 Opened 14 years ago Closed 14 years ago

Thunderbird mozmill test: test_escape_rules failing following bug 564669 landing on trunk (Remove nsIPlaintextEditor::handleKeyPress())

Categories

(Thunderbird :: Mail Window Front End, defect)

Product:
Thunderbird
Component:
Mail Window Front End
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: standard8, Assigned: asuth)

References

Details

(Keywords: regression)

Attachments

(1 file)

automated test v1.0
4.49 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
Since bug 564669 landed, this mozmill test has been failing. TEST-UNEXPECTED-FAIL | test_escape_rules EXCEPTION: addrbook's checked state should be false at: test-quick-filter-bar-helper.js line 139 Error("addrbook's checked state should be false") 0 assert_constraints_expressed([object Object]) test-quick-filter-bar-helper.js 139 legwork() test-keyboard-interface.js 92 I'm not quite sure how the editor could affect this, hopefully asuth can offer some insight here as he wrote the test.
The test is focusing a XUL textbox with type="search" that has no text in it. It generates a synthetic keypress of VK_ESCAPE using mozmill's keypress method. Because there is nothing entered in the textbox, we expect the escape key to bubble and end up being handled by the window's controller which has a command associated with VK_ESCAPE. With bug 564669's patch, it appears that this bubbling to the controller is no longer happening. Running Thunderbird for real, hitting the escape key with the search box focused does not have the desired effect. Focusing something else (thread pane, empty browser frame) and hitting escape does result in the right thing happening. It looks like the new code adds a "aKeyEvent->PreventDefault();" that was not there before. Should that really be there?
Which XUL file is that?
Oh, this looks pretty serious. Masayuki, why should editable fields eat this Escape keypress? This can break a lot of XUL and web content.
Boy, this is *really* bad. Try loading data:text/html,<script>prompt("foo");</script> in a page, and press Esc when the dialog shows up. It works before the fix to bug 564669, it doesn't work after that.
And the fix to this bug requires tests, seriously!
Flags: in-testsuite?
(In reply to comment #4) > Oh, this looks pretty serious. Masayuki, why should editable fields eat this > Escape keypress? This can break a lot of XUL and web content. Um, this bug means Tb is killing an editor feature unexpectedly, right? ESC key is used for canceling the input after the editor gets focus. I don't think my patch is wrong for this issue. Tb should catch the ESC key event in capture phase rather than bubbling phase because tb wants to kill the editor feature. However, if the change has very big impact, I'll land the patch without PreventDefault() in ESC key handling. But that means that ESC key works on editor but the ESC key press event also do another work.
I think that additional cause of this bug is that even if an editor does nothing by ESC key press event, the editor consumes the event always.
(In reply to comment #8) > I think that additional cause of this bug is that even if an editor does > nothing by ESC key press event, the editor consumes the event always. Right. 'We' expect that the VK_ESCAPE event will bubble out if the text field didn't do anything with it. If the text field did clear itself, then we don't expect the event to bubble out. Thunderbird is not trying to alter the behaviour of the text field or the XUL widget that embeds the text field. (And Thunderbird is not directly listening for VK_ESCAPE itself; we are using the XUL controller mechanism and have a "key" associated with VK_ESCAPE.) Here's the XUL widget code we are using's special escape logic, which might be relevant: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/textbox.xml#408
Thank you. I'll post a new patch which doesn't prevent the ESC keypress events in bug 564669. And I'll file a new bug for ESC key handling.
Bug 570455 re-landed and this is all still passing fine. Thanks everyone.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment on attachment 452627 [details] [diff] [review] automated test v1.0 The test looks good, but could you please switch to waitForFocus instead of adding event listeners for load and focus events? r=me with that.
Attachment #452627 - Flags: review?(ehsan) → review+
(In reply to comment #13) > (From update of attachment 452627 [details] [diff] [review]) > The test looks good, but could you please switch to waitForFocus instead of > adding event listeners for load and focus events? No, I couldn't use it. waitForFocus() failed by permission problem.
(In reply to comment #14) > (In reply to comment #13) > > (From update of attachment 452627 [details] [diff] [review] [details]) > > The test looks good, but could you please switch to waitForFocus instead of > > adding event listeners for load and focus events? > > No, I couldn't use it. waitForFocus() failed by permission problem. Hmm, that shouldn't really happen. What did your patch look like? What was the error? I really don't want us to land a new test which we know is going to turn orange at some point...
When I call SimpleTest.waitForFocus(onPromptFocus, gPromptWindow), I got following error: > Exception caught in waitForEvent: Permission denied for <http://mochi.test:8888> to call method Location.toString on <>., at: http://mochi.test:8888/tests/SimpleTest/SimpleTest.js (307) And even if this doesn't failed, when the callback method is called, the focus is in the editor on prompt window?
Er, I called the waitForFocus from WindowObserver::observe().
Even if I added netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect'); to waitForEvent(), the callback is never called.
Maybe something funky is going on here with waitForFocus. CCing Neil to get his input on this, but let's go ahead and land this patch as it is for now. Thanks!
The test causes the browser to hang, which causes mochitests to time out and kill the run after 330 seconds. This is on x86_64-Linux (Ubuntu 10.04) with the display being a 1024x768 16-bit VNC server with no window manager. It produces a text entry box, entitled "summary", and with "text" in the text entry field, highlighted, and with Cancel and OK buttons. It then appears to be waiting for a click on one of the buttons. If I do that "by hand", the test continues successfully. 94738 INFO TEST-START | /tests/editor/libeditor/text/tests/test_bug569988.html 94739 INFO TEST-INFO | /tests/editor/libeditor/text/tests/test_bug569988.html | before wait for focus -- loaded: loading active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html desired window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html child window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html docshell visible: true 94740 INFO TEST-INFO | /tests/editor/libeditor/text/tests/test_bug569988.html | must wait for load 94741 INFO TEST-INFO | /tests/editor/libeditor/text/tests/test_bug569988.html | already focused 94742 INFO TEST-INFO | /tests/editor/libeditor/text/tests/test_bug569988.html | maybe run tests <load:false, focus:true> -- loaded: loading active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html desired window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html child window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html docshell visible: true 94743 INFO TEST-INFO | /tests/editor/libeditor/text/tests/test_bug569988.html | waitForEvent called <type:load, target[object HTMLDocument]> -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html desired window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html child window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html docshell visible: true 94744 INFO TEST-INFO | /tests/editor/libeditor/text/tests/test_bug569988.html | maybe run tests <load:true, focus:true> -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html desired window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html child window: ([object Window]) http://mochi.test:8888/tests/editor/libeditor/text/tests/test_bug569988.html docshell visible: true 94745 INFO TEST-PASS | /tests/editor/libeditor/text/tests/test_bug569988.html | prompt window is created 94746 INFO TEST-PASS | /tests/editor/libeditor/text/tests/test_bug569988.html | onPromptLoad is called TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/text/tests/test_bug569988.html | application timed out after 330 seconds with no output SCREENSHOT:data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAABAAAAAMACAIA [..vast amount of text removed..] INFO | automation.py | Application ran for: 0:22:12.062029 INFO | automation.py | Reading PID log: /tmp/tmp6sxE2Gpidlog ==> process 22594 launched child process 22652 ==> process 22594 launched child process 22677 INFO | automation.py | Checking for orphan process with PID: 22652 INFO | automation.py | Checking for orphan process with PID: 22677 PROCESS-CRASH | /tests/editor/libeditor/text/tests/test_bug569988.html | application crashed (minidump found) Neither MINIDUMP_STACKWALK nor MINIDUMP_STACKWALK_CGI is set, can't process dump. WARNING | automationutils.processLeakLog() | refcount logging is off, so leaks can't be detected! INFO | runtests.py | Running tests: end. make: *** [mochitest-plain] Error 1
I wonder if this (possibly random hangs and/or focus weirdness) is somehow related to bug 583699. It concerns the exact same test.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: