TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_nonempty_reply_above TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_empty_reply_below TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_nonempty_reply_below TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_forward TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_insertion_restores_caret_point First seen after the M-C merge this morning. M-C last good: 6190b1e65676010bc61ca612b14198992a M-C first bad: 7c637da65c1fec33fc36a014485acad541 https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6190b1e65676010bc61ca612b14198992a&tochange=7c637da65c1fec33fc36a014485acad541 https://archive.mozilla.org/pub/thunderbird/nightly/2018/01/2018-01-27-03-02-01-comm-central/comm-central_win7_ix_test-mozmill-bm111-tests1-windows-build191.txt.gz First error in the log is: INFO - SUMMARY-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_nonempty_reply_above INFO - EXCEPTION: a != b: 'This is a line of textand here's another!' != 'and here's another!'. INFO - at: test-folder-display-helpers.js line 2917 INFO - assert_true test-folder-display-helpers.js:2917 11 INFO - assert_equals test-folder-display-helpers.js:2904 3 INFO - subtest_adding_filelinks_to_reply_above_plaintext test-cloudfile-attachment-urls.js:462 5 The test "types" two lines, defined in var kLines = ["This is a line of text", "and here's another!"]; and later tests for the second of those lines } else { let targetText = aText[aText.length - 1]; <== last line let textNode = br.previousSibling; assert_equals(textNode.nodeType, kTextNodeType); assert_equals(textNode.nodeValue, targetText); <== failing line 462 I haven't run the test locally, but it seems to come from: 34abf9793163 Masayuki Nakano - Bug 1433101 - part 2: Treat Enter and Shift+Enter as printable key r=smaug 81157078b090 Masayuki Nakano - Bug 1433101 - part 1: Add new pref which disables keypress event for non-printable keys only for the default event group in web content r=smaug Somehow our way to enter a new line stopped working in Mozmill. I can still create newlines in a Daily with this change, so maybe we need to adjust Mozmill. Another one for Aceman, so we don't get bored. Any hints from author or reviewer of bug 1433101?

I watched the test now and both lines "This is a line of text" and "and here's another!" are entered without newline in between. So we need to tweak the function that does the typing.

Not sure why this stopped working: function type_in_composer(aController, aText) { // If we have any typing to do, let's do it. let frame = aController.eid("content-frame"); for (let [i, aLine] of aText.entries()) { aController.type(frame, aLine); if (i < aText.length - 1) aController.keypress(frame, "VK_RETURN", {}); } }

(In reply to Jorg K (GMT+1) from comment #2) > function type_in_composer(aController, aText) { > // If we have any typing to do, let's do it. > let frame = aController.eid("content-frame"); > for (let [i, aLine] of aText.entries()) { > aController.type(frame, aLine); Interestingly .type(aLine + "\n") makes a proper newline.

Based on Aceman's investigation, this makes all the tests pass for me.

I'm going to land that one-line hack now, happy to back it out for a better solution.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/8e9463f5d78a type \n instead of sending VK_RETURN. rs=bustage-fix

Perhaps, it doesn't generate correct Enter key event. I guess it doesn't set KeyboardEvent.key value to "Enter", but just set KeyboardEvent.keyCode to KeyboardEvent.DOM_VK_RETURN.

I got it. https://searchfox.org/comm-central/source/mail/test/resources/mozmill/mozmill/extension/resource/stdlib/EventUtils.js#338-342,345 mozmill's EventUtils still uses obsolete API, nsIDOMWindowUtils.sendKeyEvent(). related to bug 1134540 and bug 1134542. It should be rewritten with new API, nsITextInputProcessor like EventUtils for Mochitest: https://searchfox.org/mozilla-central/rev/11d0ff9f36465ce19b0c43d1ecc3025791eeb808/testing/mochitest/tests/SimpleTest/EventUtils.js#873-905

Comment on attachment 8946095 [details] [diff] [review] 1433648-newline.patch [landed in comment #6 as a temporary fix] Thanks Masayuki-san for the research. We'll switch to the new interface then (and backout the hack).

FYI: Using nsITextInputProcess means that keypress event is dispatched only when it's necessary. E.g., if preceding keydown event's default is prevented, keypress event is automatically not dispatched. If given event is a modifier key, keypress event is never fired. So, if some tests expect such illegal keypress event, you need to fix such tests too. But I guess that it must be rare.

>if preceding keydown event's default is prevented, keypress event is automatically not dispatched. It seems to me this is also what the current mozmill code honors.

I'll remove nsIDOMWindowUtils.sendKeyEvent(). So, please update mozmill's EventUtils.js. (see also bug 1434317)

Thanks for the tip. Using the m-c code seems to work, fixes this test and produces no regressions in other tests: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d6a8a7242e2fe50e4203d50dfd8f7ef6e3f6ca13 I just copied the needed functions. The only changes are the _EU_{cc,ci} which I replaced with the real Components.* because somehow setting the attributes on 'window' didn't work ('window' wasn't found).

Comment on attachment 8947182 [details] [diff] [review] replace nsIDOMWindowUtils.sendKeyEvent() Review of attachment 8947182 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for fixing this. Not much to review here if this is a (slightly modified) copy of M-C code. I'd say: rs=jorgk. ::: mail/test/resources/mozmill/mozmill/extension/resource/stdlib/EventUtils.js @@ +316,5 @@ > * a keydown, a keypress and then a keyup event are fired in sequence. > * > * aWindow is optional, and defaults to the current window object. > */ > +function synthesizeKey(aKey, aEvent, aWindow = window, aCallback) What's the story with the 'aCallback' parameter? Are there any callers providing it? @@ +831,5 @@ > > return utils.sendSelectionSetEvent(aOffset, aLength, aReverse); > } > + > +/* This block copied from mozilla-central/testing/mochitest/tests/SimpleTest/EventUtils.js */ Nit: Perhaps add a verb here, like "was".

(In reply to Jorg K (GMT+1) from comment #14) > mail/test/resources/mozmill/mozmill/extension/resource/stdlib/EventUtils.js > @@ +316,5 @@ > > * a keydown, a keypress and then a keyup event are fired in sequence. > > * > > * aWindow is optional, and defaults to the current window object. > > */ > > +function synthesizeKey(aKey, aEvent, aWindow = window, aCallback) > > What's the story with the 'aCallback' parameter? Are there any callers > providing it? I don't know, we probably do not provide it. I didn't want to change the original code too much by removing it. We may also need it in the future.

Comment on attachment 8947182 [details] [diff] [review] replace nsIDOMWindowUtils.sendKeyEvent() Sure, if new lines are just copied from EventUtils.js of mochitest, looks fine. And perhaps, you should copy the explanation of synthesizeKey() from mochitest's EventUtils.js too. When I make it use nsITextInputProcessor, I added several features to it. So, other developers should read the comment when they use it.

Good idea, thanks.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/6a1bb92d2e92 Convert mozmill's EventUtils.js from the obsolete nsIDOMWindowUtils.sendKeyEvent() API to nsITextInputProcessor. r=jorgk

I've taken the liberty to make this comment block nicer: https://hg.mozilla.org/comm-central/rev/6a1bb92d2e92#l2.126