Bug 88502

Summary: [Shadow][Editing] applying document.execCommand('bold') twice to elements having shadow insertion points causes a crash.
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: HTML EditingAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, dglazkov, dominicc, enrica, hayato, morrita, rniwa, tasak, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 82697    
Attachments:
Description Flags
Repro
none
Patch
none
Patch none

Shinya Kawanaka
Reported 2012-06-06 22:46:12 PDT
HTML: <div contenteditable> BEFORE HOST <div id="host"> <span contenteditable="false">not editable<span></div> AFTER HOST </div> Shadow DOM for host: <div id="shadow-host"><span contenteditable>SHADOW (BEFORE)</span><shadow></shadow>SHADOW (AFTER)</div> Shadow DOM for shadow-host <div contenteditable>NESTED BEFORE<shadow></shadow><input></input>NESTED AFTER</div> Select from NESTED BEFORE to NESTED AFTER, then document.execCommand('bold') twice. It causes a crash. ASSERTION FAILED: !m_insertedChild->parentNode() InsertNodeBeforeCommand.cpp(41) : InsertNodeBeforeCommand
Attachments
Repro (1.45 KB, text/html)
2012-06-11 22:49 PDT, Shinya Kawanaka 		no flags
Details
Patch (6.71 KB, patch)
2012-06-18 15:04 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (7.62 KB, patch)
2012-06-19 11:15 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2012-06-11 00:36:01 PDT
Hmm... Currently "not editable" becomes bold after the first document.execCommand('bold'). It's strange.
Shinya Kawanaka
Comment 2 2012-06-11 00:36:23 PDT
Anyway, I'll investigate this more.
Shinya Kawanaka
Comment 3 2012-06-11 00:37:20 PDT
Maybe this is related to Bug 88483...
Shinya Kawanaka
Comment 4 2012-06-11 22:49:53 PDT
Created attachment 147009 [details] Repro Place it in LayoutTest/editing/shadow/
Shinya Kawanaka
Comment 5 2012-06-13 19:06:07 PDT
This will be happend when <shadow> is removed from a tree. It will invalidate Shadow DOM distribution, so renderer will be gone. However, we still need the renderer. We have to recalculate layout again. Bug 88968 will resolve this.
Shinya Kawanaka
Comment 6 2012-06-18 14:23:38 PDT
Actually this is very related to Bug 89380. To fix this, some subset of Node::rendererIsEditable should be converted to Node::isContentEditable().
Shinya Kawanaka
Comment 7 2012-06-18 15:04:44 PDT
Created attachment 148181 [details] Patch
Ryosuke Niwa
Comment 8 2012-06-18 15:18:05 PDT
Comment on attachment 148181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148181&action=review > Source/WebCore/ChangeLog:9 > + When inserting or removing insertion points (<shadow> or <content>) into or from Shdaow DOM, shadow content > + distribution is invalidated. It will remove all renderers in the same Shadow DOM. When it happends in editing, What do you mean by "shadow content distribution" is invalidated? > Source/WebCore/ChangeLog:10 > + a content-editable elements may be considered as non-content-editable element. This triggers ASSERT failure, since Why? We need to clarify what's happening. e.g. style isn't updated properly. > Source/WebCore/ChangeLog:13 > + To prevent it, content-editablility should be recalculated if necessary. This will be achieved by using To prevent what? > LayoutTests/editing/shadow/bold-twice-in-shadow-expected.txt:3 > +before host > + not editable > +after host Can we hide this? > LayoutTests/editing/shadow/bold-twice-in-shadow-expected.txt:7 > +PASS document.execCommand("Bold") did not cause a crash. > +PASS successfullyParsed is true > + > +TEST COMPLETE This output isn't helpful because we can't tell what we're tesing. > LayoutTests/editing/shadow/bold-twice-in-shadow.html:34 > +debug('PASS document.execCommand("Bold") did not cause a crash.'); > + > +var successfullyParsed = true; I don't think we need to make this a js test. All we need is document.write.
Shinya Kawanaka
Comment 9 2012-06-19 11:15:18 PDT
Created attachment 148365 [details] Patch
WebKit Review Bot
Comment 10 2012-06-19 13:14:06 PDT
Comment on attachment 148365 [details] Patch Clearing flags on attachment: 148365 Committed r120749: <http://trac.webkit.org/changeset/120749>
WebKit Review Bot
Comment 11 2012-06-19 13:14:23 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.