Closed Bug 1601585 Opened 8 months ago Closed 6 months ago

Intermittent editor/libeditor/tests/test_bug622371.html | The start offset of the selection shouldn't change - got +0, expected 1

Categories

(Core :: DOM: Editor, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox-esr68 --- unaffected
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: masayuki)

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Filed by: nerli [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=279745152&repo=autoland
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/VGOqccGLSyKvqiZMZw8kpg/runs/0/artifacts/public/logs/live_backing.log


[task 2019-12-05T09:55:09.024Z] 09:55:09 INFO - TEST-START | editor/libeditor/tests/test_bug622371.html
[task 2019-12-05T09:55:09.025Z] 09:55:09 INFO - GECKO(3148) | [Child 3150, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /builds/worker/workspace/build/src/dom/base/Document.cpp, line 5041
[task 2019-12-05T09:55:09.063Z] 09:55:09 INFO - GECKO(3148) | [Parent 3148, Main Thread] WARNING: we only accept nsIURI interface type, patch welcome: file /builds/worker/workspace/build/src/dom/ipc/PropertyBagUtils.cpp, line 112
[task 2019-12-05T09:55:09.228Z] 09:55:09 INFO - TEST-INFO | started process screencapture
[task 2019-12-05T09:55:09.364Z] 09:55:09 INFO - TEST-INFO | screencapture: exit 0
[task 2019-12-05T09:55:09.364Z] 09:55:09 INFO - TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug622371.html | The start offset of the selection shouldn't change - got +0, expected 1
[task 2019-12-05T09:55:09.364Z] 09:55:09 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:322:16
[task 2019-12-05T09:55:09.364Z] 09:55:09 INFO - @editor/libeditor/tests/test_bug622371.html:32:5
[task 2019-12-05T09:55:09.364Z] 09:55:09 INFO - rval@SimpleTest/SimpleTest.js:146:30
[task 2019-12-05T09:55:09.364Z] 09:55:09 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-12-05T09:55:09.364Z] 09:55:09 INFO - TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug622371.html | The end offset of the selection shouldn't change - got +0, expected 1
[task 2019-12-05T09:55:09.365Z] 09:55:09 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:322:16
[task 2019-12-05T09:55:09.365Z] 09:55:09 INFO - @editor/libeditor/tests/test_bug622371.html:33:5
[task 2019-12-05T09:55:09.365Z] 09:55:09 INFO - rval@SimpleTest/SimpleTest.js:146:30
[task 2019-12-05T09:55:09.365Z] 09:55:09 INFO - GECKO(3148) | MEMORY STAT | vsize 7396MB | residentFast 219MB | heapAllocated 67MB
[task 2019-12-05T09:55:09.365Z] 09:55:09 INFO - TEST-OK | editor/libeditor/tests/test_bug622371.html | took 229ms
[task 2019-12-05T09:55:09.365Z] 09:55:09 INFO - GECKO(3148) | [Parent 3148, Main Thread] WARNING: we only accept nsIURI interface type, patch welcome: file /builds/worker/workspace/build/src/dom/ipc/PropertyBagUtils.cpp, line 112

Oh, this orange detects actual inconsistent behavior at load event.

When it's green, HTMLEditor has already been created for contenteditable. Then, Selection::Collapse() is performed and HTMLEditor::BegginingOfDocument() is not called at turning designMode on because editing state was not off. However when it's orange, there is no selection ranges because contenteditable has not been handled yet (i.e., the editing state is still off at setting designMode to on).

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
OS: Unspecified → All
Priority: P5 → P3
Hardware: Unspecified → All
Version: unspecified → Trunk

We should port editor/libeditor/tests/test_bug622371.html to WPT. It was
written for compatibility issue with CKEditor 9 years ago and Gecko and
Blink behave as exactly same. Only Safari normalize the selection into the
end of the text node, but Safari also does not change selection at turning
on/off designMode too. Therefore, all browser engines have implicit
agreement at least that selection shouldn't be changed at changing
designMode.

For backward compatibility, Document::EditingStateChanged() calls
HTMLEditor::BeginingOfDocument() to initialize selection when the document
becomes editable first time. However, it checks whether
Document::mEditingState was ``EditingState::eOffor not. This looks enough, but not so becauseDocument::MaybeEditingStateChanged()` calls it
asynchronously if `contenteditable` element appears at not safe to run script.
Therefore, this patch makes it check `Document::mContentEditableCount` value
which is modified synchronously from `Element::AfterSetAttr()` (additionally,
this makes it check whether there is at least a range in normal selection too,
though).

Depends on D62813

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/331fac3d22bc
Add a WPT to test selection in a `contenteditable` element with changing `designMode` r=smaug
https://hg.mozilla.org/integration/autoland/rev/313ca1b7d400
Make `Document::EditingStateChanged()` stop calling `HTMLEditor::BeginingOfDocument()` when there is a pending state change for `contenteditable` elements and there is a selection range r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/21817 for changes under testing/web-platform/tests
Flags: needinfo?(masayuki)
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75

Is this something we should consider for Beta backport?

Upstream PR was closed without merging
Upstream PR merged by moz-wptsync-bot

(In reply to Ryan VanderMeulen [:RyanVM] from comment #20)

Is this something we should consider for Beta backport?

I think that this should ride on the train because I guess that it must be rare web apps using designMode (legacy API) to include contenteditable (modern API than the previous one). On the other hand, if there are some regressions of this change, it must appear on Thunderbird since the BeginningOfDocument call is required mainly for it. Additionally, the symptom is not reported for existing web apps.

Flags: needinfo?(masayuki)
You need to log in before you can comment on or make changes to this bug.