Summary: | Deleting a content-editable element having a ShadowRoot causes a crash | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Shinya Kawanaka <shinyak> | ||||||
Component: | DOM | Assignee: | 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
Shinya Kawanaka
2012-05-14 02:14:55 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... (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 :( (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. Created attachment 145724 [details]
Patch
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. Created attachment 146209 [details]
Patch for landing
Comment on attachment 146209 [details] Patch for landing Clearing flags on attachment: 146209 Committed r119711: <http://trac.webkit.org/changeset/119711> All reviewed patches have been landed. Closing bug. |