Please see the uploaded html file.

This also crashes in opt, in the spellchecker code...

So I just took a quick look, the issue is that spell-checking code accepts shadow-including descendants: https://searchfox.org/mozilla-central/rev/80ac71c1c54af788b32e851192dfd2de2ec18e18/extensions/spellcheck/src/mozInlineSpellChecker.cpp#1367 But then the SetEnd call right below that line ends up looking for the next node in the light DOM using GetNextNode(): https://searchfox.org/mozilla-central/rev/80ac71c1c54af788b32e851192dfd2de2ec18e18/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp#143 Which crashes if they're not in the same light DOM subtree. Furthermore all the offsets the spellchecking code maintains are referring to DOM children, not flattened tree children... So probably somebody needs to go ahead and figure out how this should really work with Shadow DOM... If this kinda works right now we may want to just wallpaper the crash for now, but... Olli, do you know who's familiar with this code and could help?

edgar, do you have spare cycles? If not, I'll take a look soon.

I will take a look.

I found other issues in spell-checking with Shadow DOM, too, for example, - Full range checking doesn't check the text inside the shadow DOM. - Navigate in/out Shadow DOM doesn't trigger spell-checking. - I saw a lot of We could teach mozInlineSpellWordUtil about the flattened tree which could fix this crash, it doesn't make spell-checking be fully compatible with Shadow DOM. There is still some other part we need to figure out how to deal with Shadow DOM, for example, spell-checking code use nsRange and nsRange only deal with subtree. But at least, we don't crash.

I'm not really expecting spell checking to cross shadow boundary.

Pushed by echen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/31d9a680fb90 Part 1: Set the root for spelling checker to shadow root if the contenteditable nodes are in the shadow DOM; r=smaug https://hg.mozilla.org/integration/autoland/rev/cf74ff09b141 Part 2: Make spellchecking to work in ShadowDOM after anchor navigates away from it; r=smaug https://hg.mozilla.org/integration/autoland/rev/6c2a5ce9fad8 Part 3: Add tests for spell-checking in ShadowDOM; r=smaug

Backed for causing Android mochitest failures on editor/libeditor/tests/test_bug Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=6c2a5ce9fad8b976bfa51fdc80908d1bbfd0e90f&tochange=4610b4c5befa0f89360ab554efcf0dbd9a1e0a07&searchStr=android%2C4.3%2Capi16&selectedJob=208933385 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=208933385&repo=autoland&lineNumber=1472 Backout link: https://hg.mozilla.org/integration/autoland/rev/e2897d099d2a6e78599312f386b8a80352c3c506 [task 2018-10-31T16:53:05.320Z] 16:53:05 INFO - 84 INFO TEST-START | editor/libeditor/tests/test_bug1497480.html [task 2018-10-31T16:53:15.641Z] 16:53:15 INFO - Buffered messages logged at 16:53:03 [task 2018-10-31T16:53:15.641Z] 16:53:15 INFO - 85 INFO AddTask.js | Entering test [task 2018-10-31T16:53:15.642Z] 16:53:15 INFO - 86 INFO AddTask.js | Leaving test [task 2018-10-31T16:53:15.642Z] 16:53:15 INFO - 87 INFO AddTask.js | Entering test [task 2018-10-31T16:53:15.642Z] 16:53:15 INFO - Buffered messages finished [task 2018-10-31T16:53:15.642Z] 16:53:15 INFO - 88 INFO TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug1497480.html | Correct number of misspellings and words. - got +0, expected 1 [task 2018-10-31T16:53:15.642Z] 16:53:15 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:312:5 [task 2018-10-31T16:53:15.642Z] 16:53:15 INFO - isSpellingCheckOk@editor/libeditor/tests/spellcheck.js:6:3 [task 2018-10-31T16:53:15.642Z] 16:53:15 INFO - @editor/libeditor/tests/test_bug1497480.html:69:10

Okay, I found other spell-checking tests are all disabled on android, maybe it is doesn't work well on android. Let's just disable this on android.

Pushed by echen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d300e4c01708 Part 1: Set the root for spelling checker to shadow root if the contenteditable nodes are in the shadow DOM; r=smaug https://hg.mozilla.org/integration/autoland/rev/c094ca2c2a1d Part 2: Make spellchecking to work in ShadowDOM after anchor navigates away from it; r=smaug https://hg.mozilla.org/integration/autoland/rev/742633251297 Part 3: Add tests for spell-checking in ShadowDOM; r=smaug

https://hg.mozilla.org/mozilla-central/rev/d300e4c01708 https://hg.mozilla.org/mozilla-central/rev/c094ca2c2a1d https://hg.mozilla.org/mozilla-central/rev/742633251297

Is this something we should consider for Beta backport or can it ride the trains?

IMO, we should consider for Beta backport.

I agree beta uplift. It causes crash, and the current beta is at the beginning of its cycle.

Edgar, could you ask approvals for beta?

Comment on attachment 9019390 [details] Bug 1497480 - Part 1: Set the root for spelling checker to shadow root if the contenteditable nodes are in the shadow DOM; [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: None User impact if declined: Browser crash if user press enters in contenteditable elements which are in the shadow DOM. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Low risk. Patches here is to add safe handling for shadow DOM to avoid spell-checker crossing shadow boundary. String changes made/needed: None

Comment on attachment 9019391 [details] Bug 1497480 - Part 2: Make spellchecking to work in ShadowDOM after anchor navigates away from it; [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: None User impact if declined: Browser crash if user press enters in contenteditable elements which are in the shadow DOM. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Low risk. Patches here is to add safe handling for shadow DOM to avoid spell-checker crossing shadow boundary. String changes made/needed: None.

Comment on attachment 9020055 [details] Bug 1497480 - Part 3: Add tests for spell-checking in ShadowDOM; [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: None User impact if declined: Browser crash if user press enters in contenteditable elements which are in the shadow DOM. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): Low risk. Patches here is to add safe handling for shadow DOM to avoid spell-checker crossing shadow boundary. String changes made/needed: None.

Comment on attachment 9019390 [details] Bug 1497480 - Part 1: Set the root for spelling checker to shadow root if the contenteditable nodes are in the shadow DOM; crash fix, regression in 64, approved for 64.0b7

https://hg.mozilla.org/releases/mozilla-beta/rev/fa8dfa3aef26 https://hg.mozilla.org/releases/mozilla-beta/rev/ad24ecd2dd39 https://hg.mozilla.org/releases/mozilla-beta/rev/f9639e65bc99