1652557 - Fix failing DOM editor mochitests with cross-origin and Fission enabled

Status: RESOLVED FIXED

(Core :: DOM: Editor, task, P1)

Description

[Neha Kochar [:neha]]

These tests are marked as skipped/failed for cross-origin and Fission, and need to be fixed
editor/libeditor/tests/test_bug767684.html
editor/libeditor/tests/test_resizers_resizing_elements.html

https://docs.google.com/spreadsheets/d/16G5AZhHWWow3rBgim4QBHzWXMIIJiky2SzXYgDMTTKY/edit#gid=1354562828&range=20:22

More info: https://wiki.mozilla.org/Project_Fission/Enabling_Tests_with_Fission#Cross-Origin_Mochitests

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Mirko, can you please take care of this (after your vacation)? Thanks!

Comment 2

[Mirko Brodesser (:mbrodesser-Igalia)]

I'll take a first look at this when I've flushed out all pending changes for the other P1 bug I'm assigned too.

Comment 3

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1652557: don't skip test_bug767684.html when xorigin iframes and fission are enabled. r=masayuki

The test failed neither locally nor on try, hence enabling it again.

Comment 4

[Mirko Brodesser (:mbrodesser-Igalia)]

Neha: according to the spreadsheet linked in c1, test_bug772796.html should be part of this bug ticket, right?

Comment 5

[Neha Kochar [:neha]]

Yes, these are the tests for DOM: Editor that need to be fixed:
editor/libeditor/tests/test_bug767684.html
editor/libeditor/tests/test_resizers_resizing_elements.html
editor/libeditor/tests/test_bug772796.html
editor/libeditor/tests/test_bug1258085.html

These are additional comments in the spreadsheet that might be helpful.

Comment 6

[Mirko Brodesser (:mbrodesser-Igalia)]

Some findings about editor/libeditor/tests/test_resizers_resizing_elements.html:

1. It randomly fails when resizing <img>, <div> or <table> elements.
2. The pref value of "editor.resizing.preserve_ratio" doesn't matter, it sometimes fails when it's true and sometimes when it's false.
3. The test case can be reduced by removing the event listeners for "beforeinput" and "input" and it still fails sometimes.
4. The test still fails when not setting "dom.input_events.beforeinput.enabled" to true.
5. Running the test only with --enable-xorigin-tests (without --enable-fission) doesn't seem to cause the failures.
6. Sleeping for 100 ms (using setTimeout) after all synthesizeMouse... calls after clicking a resizer doesn't help.
7. Sleeping for 100 ms (using setTimeout) after all synthesizeMouse... calls after clicking a resizer without --enable-xorigin-tests --enable-fission causes the failures too. This wasn't reproducible anymore. I suspect a physical, unintended mouse-move might have caused the failure.
8. Sometimes the first test (first call of testResizers) fails.
9. Not stopping to propagate mouse* events still let's the test fail (only with --enable-xorigin-tests --enable-fission).
10. Sleeping (like in 6)) after the first synthesizeMouseAtCenter call leads to sporadic failures.
11. The failure was reproducible when waiting for 2 animation frames after resetting width and height, and after all synthesize* calls.
12. When is(target, document.activeElement, "target is active element."); added after clicking on the target to be resized, it always fails, even when the remainder of the test passes.
13. The tests uses enableObjectResizing, which isn't supported by Chrome and isn't spec'ed.
14. With --enable-xorigin-tests --enable-fission after synthesizing mouse clicks, unexpected onblur and onfocus occur.

7. implies this is not a fission-specific problem.
8. implies it's not a problem of one test not cleaning up after its execution.
14. indicates, for fission with xorigin iframes enabled, the problem is likely the same as in bug 1653160.

Searching in the dark:
a) In case of failure, HTMLEditor::OnMouseUp is called twice with mIsResizing == false immediately before the failure. In all success cases, it's called first with mIsResizing == true and then with mIsResizing == false, so perhaps the problem is around that variable.
b) In the failure case, HTMLEditor::OnMouseDown andHTMLEditor::StartResizing aren't called.
c) In a failing run, HTMLEditorEventListener::MouseDown returns early, because
IsAcceptableMouseEvent returns false. It's because HTMLEditor::IsAcceptableInputEvent returns false, because EditorBase::IsAcceptableInputEvent returns false because there's no focused content. There's no focused content according to the FocusManager. In a passing run, there is focused content.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60e7f53ef698
don't skip test_bug767684.html when xorigin iframes and fission are enabled. r=masayuki

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/60e7f53ef698

Comment 9

[Mirko Brodesser (:mbrodesser-Igalia)]

(In reply to Neha Kochar [:neha] from comment #5)

Yes, these are the tests for DOM: Editor that need to be fixed:
editor/libeditor/tests/test_bug767684.html
editor/libeditor/tests/test_resizers_resizing_elements.html
editor/libeditor/tests/test_bug772796.html
editor/libeditor/tests/test_bug1258085.html

These are additional comments in the spreadsheet that might be helpful.

@neha: the last two tests are already enabled. The spreadsheet has their statuses declared as "skipped", perhaps it's worth updating.

Comment 10

[Neha Kochar [:neha]]

Yes, the spreadsheet has been updated now. Thanks for the check.

Comment 11

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1652557: remove failure expectation for "test_resizers_resizing_elements.html" when xorigin iframes and fission are enabled. r=hsivonen

Fixed by bug 1653160.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d742958c1ee0
remove failure expectation for "test_resizers_resizing_elements.html" when xorigin iframes and fission are enabled. r=hsivonen

Comment 13

[Cosmin Sabou [:CosminS]]

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