Title: [120749] trunk
- Revision
- 120749
- Author
- [email protected]
- Date
- 2012-06-19 13:13:45 -0700 (Tue, 19 Jun 2012)
Log Message
[Shadow][Editing] applying document.execCommand('bold') twice to elements having shadow insertion points causes a crash.
https://bugs.webkit.org/show_bug.cgi?id=88502
Reviewed by Ryosuke Niwa.
Source/WebCore:
When inserting or removing insertion points (<shadow> or <content>) into or from Shdaow DOM,
ElementShadow::invalidateDistribution() will be called immediately. It will remove all the renderers
of elements in the Shadow DOM. Since Node::rendererIsEditable() returns false when an renderer does not
exist, all the elements in the Shadow DOM will be considered as non-content-editable until recalculating
layout, though some of them may actually be content-editable.
Actually the current code does not recalculate layout inside editing command, so a disaster happens.
For example, performing an editing command quits before completing all the commands, because the command
thinks it is adding some elements to a non-content-editable element (but actually it's content-editable).
So we have to recalculate layout if necessary when checking an element is content-editable or not.
This can be achieved by using Node::isContentEditable() instead of Node::rendererIsEditable().
Test: editing/shadow/bold-twice-in-shadow.html
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::removeInlineStyleFromElement):
* editing/InsertNodeBeforeCommand.cpp:
(WebCore::InsertNodeBeforeCommand::doApply):
(WebCore::InsertNodeBeforeCommand::doUnapply):
* editing/RemoveNodeCommand.cpp:
(WebCore::RemoveNodeCommand::doApply):
LayoutTests:
* editing/shadow/bold-twice-in-shadow-expected.txt: Added.
* editing/shadow/bold-twice-in-shadow.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (120748 => 120749)
--- trunk/LayoutTests/ChangeLog 2012-06-19 20:11:42 UTC (rev 120748)
+++ trunk/LayoutTests/ChangeLog 2012-06-19 20:13:45 UTC (rev 120749)
@@ -1,3 +1,13 @@
+2012-06-19 Shinya Kawanaka <[email protected]>
+
+ [Shadow][Editing] applying document.execCommand('bold') twice to elements having shadow insertion points causes a crash.
+ https://bugs.webkit.org/show_bug.cgi?id=88502
+
+ Reviewed by Ryosuke Niwa.
+
+ * editing/shadow/bold-twice-in-shadow-expected.txt: Added.
+ * editing/shadow/bold-twice-in-shadow.html: Added.
+
2012-06-19 Jon Honeycutt <[email protected]>
Remove some failing layout tests results added in
Added: trunk/LayoutTests/editing/shadow/bold-twice-in-shadow-expected.txt (0 => 120749)
--- trunk/LayoutTests/editing/shadow/bold-twice-in-shadow-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/shadow/bold-twice-in-shadow-expected.txt 2012-06-19 20:13:45 UTC (rev 120749)
@@ -0,0 +1,6 @@
+Applying document.execCommand('Bold') to elements having insertion points (<shadow> or <content>) shoud not cause a crash.
+
+To test manually, make a selection from somewhere in "nested before" to somehwere in "nested after", then press Ctrl+B twice. It should not cause a crash.
+
+PASS
+
Added: trunk/LayoutTests/editing/shadow/bold-twice-in-shadow.html (0 => 120749)
--- trunk/LayoutTests/editing/shadow/bold-twice-in-shadow.html (rev 0)
+++ trunk/LayoutTests/editing/shadow/bold-twice-in-shadow.html 2012-06-19 20:13:45 UTC (rev 120749)
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script src=""
+
+<p>Applying document.execCommand('Bold') to elements having insertion points (<shadow> or <content>) shoud not cause a crash.</p>
+<p>To test manually, make a selection from somewhere in "nested before" to somehwere in "nested after", then press Ctrl+B twice. It should not cause a crash.</p>
+
+<div id="container" contenteditable>
+ before host
+ <div id="host"> <span contenteditable="false">not editable<span></div>
+ after host
+</div>
+<pre id="console"></pre>
+
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var shadowRoot = new WebKitShadowRoot(host);
+shadowRoot.innerHTML = '<div id="shadow-host"><span contenteditable>shadow before</span><shadow></shadow>shadow (after)</div>';
+
+var nestedShadowHost = shadowRoot.getElementById('shadow-host');
+var nestedShadowRoot = new WebKitShadowRoot(nestedShadowHost);
+nestedShadowRoot.innerHTML = '<div id="inner" contenteditable>nested before<shadow></shadow>nested after</div>';
+
+if (window.eventSender) {
+ var div = nestedShadowRoot.getElementById('inner');
+
+ eventSender.mouseMoveTo(div.offsetLeft + 5, div.offsetTop + div.offsetHeight / 2);
+ eventSender.mouseDown();
+ eventSender.mouseMoveTo(div.offsetLeft + div.offsetWidth - 5, div.offsetTop + div.offsetHeight / 2);
+ eventSender.mouseUp();
+
+ document.execCommand('Bold');
+ document.execCommand('Bold');
+
+ container.innerHTML = "PASS";
+}
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (120748 => 120749)
--- trunk/Source/WebCore/ChangeLog 2012-06-19 20:11:42 UTC (rev 120748)
+++ trunk/Source/WebCore/ChangeLog 2012-06-19 20:13:45 UTC (rev 120749)
@@ -1,3 +1,33 @@
+2012-06-19 Shinya Kawanaka <[email protected]>
+
+ [Shadow][Editing] applying document.execCommand('bold') twice to elements having shadow insertion points causes a crash.
+ https://bugs.webkit.org/show_bug.cgi?id=88502
+
+ Reviewed by Ryosuke Niwa.
+
+ When inserting or removing insertion points (<shadow> or <content>) into or from Shdaow DOM,
+ ElementShadow::invalidateDistribution() will be called immediately. It will remove all the renderers
+ of elements in the Shadow DOM. Since Node::rendererIsEditable() returns false when an renderer does not
+ exist, all the elements in the Shadow DOM will be considered as non-content-editable until recalculating
+ layout, though some of them may actually be content-editable.
+
+ Actually the current code does not recalculate layout inside editing command, so a disaster happens.
+ For example, performing an editing command quits before completing all the commands, because the command
+ thinks it is adding some elements to a non-content-editable element (but actually it's content-editable).
+
+ So we have to recalculate layout if necessary when checking an element is content-editable or not.
+ This can be achieved by using Node::isContentEditable() instead of Node::rendererIsEditable().
+
+ Test: editing/shadow/bold-twice-in-shadow.html
+
+ * editing/ApplyStyleCommand.cpp:
+ (WebCore::ApplyStyleCommand::removeInlineStyleFromElement):
+ * editing/InsertNodeBeforeCommand.cpp:
+ (WebCore::InsertNodeBeforeCommand::doApply):
+ (WebCore::InsertNodeBeforeCommand::doUnapply):
+ * editing/RemoveNodeCommand.cpp:
+ (WebCore::RemoveNodeCommand::doApply):
+
2012-06-19 Kenneth Russell <[email protected]>
Unreviewed, rolling out r120744.
Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.cpp (120748 => 120749)
--- trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2012-06-19 20:11:42 UTC (rev 120748)
+++ trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2012-06-19 20:13:45 UTC (rev 120749)
@@ -821,7 +821,7 @@
{
ASSERT(element);
- if (!element->parentNode() || !element->parentNode()->rendererIsEditable())
+ if (!element->parentNode() || !element->parentNode()->isContentEditable())
return false;
if (isStyledInlineElementToRemove(element.get())) {
Modified: trunk/Source/WebCore/editing/InsertNodeBeforeCommand.cpp (120748 => 120749)
--- trunk/Source/WebCore/editing/InsertNodeBeforeCommand.cpp 2012-06-19 20:11:42 UTC (rev 120748)
+++ trunk/Source/WebCore/editing/InsertNodeBeforeCommand.cpp 2012-06-19 20:13:45 UTC (rev 120749)
@@ -48,7 +48,7 @@
void InsertNodeBeforeCommand::doApply()
{
ContainerNode* parent = m_refChild->parentNode();
- if (!parent || !parent->rendererIsEditable())
+ if (!parent || !parent->isContentEditable())
return;
ExceptionCode ec;
@@ -60,7 +60,7 @@
void InsertNodeBeforeCommand::doUnapply()
{
- if (!m_insertChild->rendererIsEditable())
+ if (!m_insertChild->isContentEditable())
return;
// Need to notify this before actually deleting the text
Modified: trunk/Source/WebCore/editing/RemoveNodeCommand.cpp (120748 => 120749)
--- trunk/Source/WebCore/editing/RemoveNodeCommand.cpp 2012-06-19 20:11:42 UTC (rev 120748)
+++ trunk/Source/WebCore/editing/RemoveNodeCommand.cpp 2012-06-19 20:13:45 UTC (rev 120749)
@@ -42,7 +42,7 @@
void RemoveNodeCommand::doApply()
{
ContainerNode* parent = m_node->parentNode();
- if (!parent || !parent->rendererIsEditable())
+ if (!parent || !parent->isContentEditable())
return;
m_parent = parent;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
