1042469 - Flatten editor/libeditor/base to editor/libeditor

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Comment 1

[(no longer active)]

Attachment: Flatten editor/libeditor/base to editor/libeditor

Comment 2

[(no longer active)]

https://tbpl.mozilla.org/?tree=Try&rev=85565c3595b6

Comment 3

[Biru [:poiru]]

Comment on attachment 8460703 [details] [diff] [review]
Flatten editor/libeditor/base to editor/libeditor

Review of attachment 8460703 [details] [diff] [review]:
-----------------------------------------------------------------

r=poiru, although the try run has Android mochitest oranges similar to those in the previous attempt (see bug 1028565 comment 11).

Comment 4

[(no longer active)]

Hmm, Masayuki, any ideas why moving things around like this breaks test_dom_input_event_on_htmleditor.html like this?  This patch effectively makes the tests in libeditor/html run before the ones currently in libeditor/base...

Comment 5

[(no longer active)]

(For reference, this test was added in bug 668606, and it does depend on the order of events to make sure inputEvent.SetCurrentTime(); is properly called.)

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

I have no idea now. The failure must be one of following three cases:

1. Delete key isn't handled as removing a character.
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html?force=1#142
2. ArrowLeft key isn't handled as moving caret to left.
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html?force=1#139
3. Space key isn't handled as inputting ASCII whitespace.
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html?force=1#130

It must be a big hit this fails only on mobile platforms...

Comment 7

[(no longer active)]

So I tested a bunch of things.  Replacing the space with an "x" doesn't help make the test pass. <https://tbpl.mozilla.org/?tree=Try&rev=438ebd6dc5c8>  Converting the delete command to a backspace make the test pass for Editor1 (which seems to suggest that VK_LEFT is the thing that didn't work properly, except that it moves the test failure *with backspace* to Editor2!!!  <https://tbpl.mozilla.org/?tree=Try&rev=fda0495db4af>

That is truly bizarre, I have no idea how to make sense of this.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

I could check it next week. At least today, I don't have much time...

Comment 9

[(no longer active)]

(In reply to comment #8)
> I could check it next week. At least today, I don't have much time...

I'd appreciate it.  :-)

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Sorry, I've never worked on this yet. I have some urgent work for now. When I work on this, I'll notify you.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Um, odd... The ArrowLeft key doesn't work.

https://tbpl.mozilla.org/?tree=Try&rev=98fe47f3ecf3

> 00:30:32     INFO -  502 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: Left key should not remove any character
> 00:30:32     INFO -  503 INFO TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Caret should be the start of the text - got 1, expected 0

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Attachment: diff for the latest m-c for Ehsan's patch

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Attachment: Add more tests into test_dom_input_event_on_htmleditor.html

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Looks like nobody consumes left key in the default event group.

capture:
https://tbpl.mozilla.org/?tree=Try&rev=ab2b6b272fd8
> 01:57:56     INFO -  505 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: input event was fired by Left key
> 01:57:56     INFO -  506 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keydown (window) keyCode=46, getPreventDefault()=false
> 01:57:56     INFO -  507 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keydown (editTarget) keyCode=46, getPreventDefault()=false
> 01:57:56     INFO -  508 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keypress (window) keyCode=46, getPreventDefault()=false
> 01:57:56     INFO -  509 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keypress (editTarget) keyCode=46, getPreventDefault()=false
> 01:57:56     INFO -  510 INFO TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: Delete key should remove nbsp - got &nbsp;, expected <br>

bubble:
https://tbpl.mozilla.org/?tree=Try&rev=d7a0589b62e5
> 02:35:48     INFO -  505 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: input event was fired by Left key
> 02:35:48     INFO -  506 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keydown (editTarget) keyCode=46, getPreventDefault()=false
> 02:35:48     INFO -  507 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keydown (window) keyCode=46, getPreventDefault()=false
> 02:35:48     INFO -  508 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keypress (editTarget) keyCode=46, getPreventDefault()=false
> 02:35:48     INFO -  509 INFO TEST-PASS | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | keypress (window) keyCode=46, getPreventDefault()=false
> 02:35:48     INFO -  510 INFO TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_dom_input_event_on_htmleditor.html | Editor1, body has contenteditable attribute: Delete key should remove nbsp - got &nbsp;, expected <br>

I'm now suspecting that it's in RTL.
https://tbpl.mozilla.org/?tree=Try&rev=e99dbecfeef6

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Ehsan, bug 1054087 is filed. I think that you should disable the test on B2G too. Then, you can land this.

Comment 16

[(no longer active)]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #15)
> Ehsan, bug 1054087 is filed. I think that you should disable the test on B2G
> too. Then, you can land this.

Thank you!  These tests seem to be disabled on b2g in the [DEFAULT] section already.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f05a36b5b90d

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f05a36b5b90d