Bug 86345

Summary: Deleting a content-editable element having a ShadowRoot causes a crash
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dominicc, enrica, hayato, morrita, rniwa, shinyak, tasak, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 82697    
Attachments:
Description Flags
Patch
none
Patch for landing none

Shinya Kawanaka
Reported 2012-05-14 02:14:55 PDT
Repro: <div id="container" contenteditable></div> <script> var shadowRoot = new WebKitShadowRoot(container); shadowRoot.innerHTML = "<span contenteditable>HOGE</span>"; </script> Select somewhere from after the container to the container. (Don't select HOGE but container.) And press DELETE.
Attachments
Patch (4.95 KB, patch)
2012-06-05 01:36 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.16 KB, patch)
2012-06-06 23:45 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2012-05-14 05:11:35 PDT
When an element does not have a shadow root, we cannot select contenteditable element in this example... So I think it is odd that contenteditable can be selected by this code...
Ryosuke Niwa
Comment 2 2012-05-14 12:09:55 PDT
(In reply to comment #1) > When an element does not have a shadow root, we cannot select contenteditable element in this example... So I think it is odd that contenteditable can be selected by this code... That's probably because we check the existence of the renderer :(
Shinya Kawanaka
Comment 3 2012-05-14 18:55:29 PDT
(In reply to comment #2) > (In reply to comment #1) > > When an element does not have a shadow root, we cannot select contenteditable element in this example... So I think it is odd that contenteditable can be selected by this code... > > That's probably because we check the existence of the renderer :( Yeah... probably so. I'll investigate this more today.
Shinya Kawanaka
Comment 4 2012-06-05 01:36:35 PDT
Created attachment 145724 [details] Patch
Ryosuke Niwa
Comment 5 2012-06-06 12:28:23 PDT
Comment on attachment 145724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145724&action=review > Source/WebCore/editing/VisibleSelection.cpp:457 > + // boundaries again. See http://wkb.ug/87463 Can we use the regular webkit bug URL instead? I don't see a benefit in using a short URL in a comment like this. > LayoutTests/editing/shadow/select-contenteditable-shadowhost.html:8 > +<p>This test confirms that selecting an element having Shadow DOM doesn't cross editing boundaries errornously.</p> Should we say something along the line of this test passes if WebKit didn't crash? > LayoutTests/editing/shadow/select-contenteditable-shadowhost.html:34 > +// Checks crash won't happen. > +document.execCommand('delete'); You should use debug() to print out something to indicate we passed the test. > LayoutTests/editing/shadow/select-contenteditable-shadowhost.html:46 > +// Check crash won't happen. > +document.execCommand('delete'); Ditto.
Shinya Kawanaka
Comment 6 2012-06-06 23:45:53 PDT
Created attachment 146209 [details] Patch for landing
WebKit Review Bot
Comment 7 2012-06-07 05:22:58 PDT
Comment on attachment 146209 [details] Patch for landing Clearing flags on attachment: 146209 Committed r119711: <http://trac.webkit.org/changeset/119711>
WebKit Review Bot
Comment 8 2012-06-07 05:23:03 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.