Changeset 64083 in webkit

Timestamp:

Jul 26, 2010 3:47:21 PM (14 years ago)

Author:

rniwa@webkit.org

Message:

Applying inline style to a text node whose parent is an inline editable root causes crash
​https://bugs.webkit.org/show_bug.cgi?id=39989

Reviewed by Darin Adler.

WebCore:

The crash was caused by splitTextElementAtStart and splitTextElementAtEnd assuming that the parent
and the grandparent of the specified text node is editable.

Modified splitTextElementAtStart and splitTextElementAtEnd so that they call splitTextAtStart
and splitTextAtEnd respectively when the grandparent is not editable.

Also modified SplitTextNodeContainingElement to exit early if the grandparent of m_text is not editable.

Test: editing/style/style-text-node-without-editable-parent.html

• editing/ApplyStyleCommand.cpp:

(WebCore::ApplyStyleCommand::splitTextElementAtStart):
(WebCore::ApplyStyleCommand::splitTextElementAtEnd):

• editing/SplitTextNodeContainingElementCommand.cpp:

(WebCore::SplitTextNodeContainingElementCommand::doApply):

LayoutTests:

Added a test to apply inline styles to a text node which is a editable root's child.
The test should not crash.

Two tests require rebaseline for the editing delegates. However, the final selection is kept same.

• editing/execCommand/hilitecolor-expected.txt:
• editing/style/remove-underline-from-stylesheet-expected.txt:
• editing/style/style-text-node-without-editable-parent-expected.txt: Added.
• editing/style/style-text-node-without-editable-parent.html: Added.
• resources/dump-as-markup.js:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/execCommand/hilitecolor-expected.txt (modified) (1 diff)
• LayoutTests/editing/style/remove-underline-from-stylesheet-expected.txt (modified) (1 diff)
• LayoutTests/editing/style/style-text-node-without-editable-parent-expected.txt (added)
• LayoutTests/editing/style/style-text-node-without-editable-parent.html (added)
• LayoutTests/resources/dump-as-markup.js (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/editing/ApplyStyleCommand.cpp (modified) (2 diffs)
• WebCore/editing/SplitTextNodeContainingElementCommand.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-07-26  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Applying inline style to a text node whose parent is an inline editable root causes crash
+        https://bugs.webkit.org/show_bug.cgi?id=39989
+
+        Added a test to apply inline styles to a text node which is a editable root's child.
+        The test should not crash.
+
+        Two tests require rebaseline for the editing delegates. However, the final selection is kept same.
+
+        * editing/execCommand/hilitecolor-expected.txt:
+        * editing/style/remove-underline-from-stylesheet-expected.txt:
+        * editing/style/style-text-node-without-editable-parent-expected.txt: Added.
+        * editing/style/style-text-node-without-editable-parent.html: Added.
+        * resources/dump-as-markup.js:
+
 2010-07-26  Martin Robinson  <mrobinson@igalia.com>
 

trunk/LayoutTests/editing/execCommand/hilitecolor-expected.txt

@@ -5 +5 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > SPAN > DIV > BODY > HTML > #document to 6 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 4 of #text > DIV > BODY > HTML > #document to 10 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > SPAN > DIV > BODY > HTML > #document to 6 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification

trunk/LayoutTests/editing/style/remove-underline-from-stylesheet-expected.txt

@@ -65 +65 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 6 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 7 of #text > DIV > BODY > HTML > #document to 0 of SPAN > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 6 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification

trunk/LayoutTests/resources/dump-as-markup.js

@@ -162 +162 @@
 
 // FIXME: Is there a better way to do this than a hard coded list?
-Markup._DUMP_AS_MARKUP_PROPERTIES = ['src', 'type', 'href', 'style', 'class', 'id', 'contentEditable'];
+Markup._DUMP_AS_MARKUP_PROPERTIES = ['src', 'type', 'href', 'style', 'class', 'id', 'color', 'bgcolor', 'contentEditable'];
 
 Markup._getAttributes = function(node)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-07-26  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Applying inline style to a text node whose parent is an inline editable root causes crash
+        https://bugs.webkit.org/show_bug.cgi?id=39989
+
+        The crash was caused by splitTextElementAtStart and splitTextElementAtEnd assuming that the parent
+        and the grandparent of the specified text node is editable.
+
+        Modified splitTextElementAtStart and splitTextElementAtEnd so that they call splitTextAtStart
+        and splitTextAtEnd respectively when the grandparent is not editable.
+
+        Also modified SplitTextNodeContainingElement to exit early if the grandparent of m_text is not editable.
+
+        Test: editing/style/style-text-node-without-editable-parent.html
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::splitTextElementAtStart):
+        (WebCore::ApplyStyleCommand::splitTextElementAtEnd):
+        * editing/SplitTextNodeContainingElementCommand.cpp:
+        (WebCore::SplitTextNodeContainingElementCommand::doApply):
+
 2010-07-26  Simon Fraser  <simon.fraser@apple.com>
 

trunk/WebCore/editing/ApplyStyleCommand.cpp

@@ -1555 +1555 @@
 void ApplyStyleCommand::splitTextElementAtStart(const Position& start, const Position& end)
 {
+    Node* parent = start.node()->parentNode();
+    if (!parent || !parent->parentElement() || !parent->parentElement()->isContentEditable())
+        return splitTextAtStart(start, end);
+
     int endOffsetAdjustment = start.node() == end.node() ? start.deprecatedEditingOffset() : 0;
     Text* text = static_cast<Text*>(start.node());
@@ -1563 +1567 @@
 void ApplyStyleCommand::splitTextElementAtEnd(const Position& start, const Position& end)
 {
+    Node* parent = end.node()->parentNode();
+    if (!parent || !parent->parentElement() || !parent->parentElement()->isContentEditable())
+        return splitTextAtEnd(start, end);
+
     Text* text = static_cast<Text*>(end.node());
     splitTextNodeContainingElement(text, end.deprecatedEditingOffset());

trunk/WebCore/editing/SplitTextNodeContainingElementCommand.cpp

@@ -49 +49 @@
 
     Element* parent = m_text->parentElement();
-    if (!parent)
+    if (!parent || !parent->parentElement() || !parent->parentElement()->isContentEditable())
         return;