Applying inline style to a text node whose parent is an inline editable root causes...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Jul 2010 22:47:21 +0000 (22:47 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Jul 2010 22:47:21 +0000 (22:47 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@64083 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/hilitecolor-expected.txt
LayoutTests/editing/style/remove-underline-from-stylesheet-expected.txt
LayoutTests/editing/style/style-text-node-without-editable-parent-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/style-text-node-without-editable-parent.html [new file with mode: 0644]
LayoutTests/resources/dump-as-markup.js
WebCore/ChangeLog
WebCore/editing/ApplyStyleCommand.cpp
WebCore/editing/SplitTextNodeContainingElementCommand.cpp

index e89dbc8..450d467 100644 (file)
@@ -1,3 +1,21 @@
+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>
 
         Reviewed by Gustavo Noronha Silva.
index d2329f7..d1aadf1 100644 (file)
@@ -4,8 +4,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 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
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
index d3b3136..826ffb6 100644 (file)
@@ -64,8 +64,7 @@ EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 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
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
diff --git a/LayoutTests/editing/style/style-text-node-without-editable-parent-expected.txt b/LayoutTests/editing/style/style-text-node-without-editable-parent-expected.txt
new file mode 100644 (file)
index 0000000..2165704
--- /dev/null
@@ -0,0 +1,289 @@
+This tests applying inline style to a text node, which is a child of the editable root. Style should be applied properly and the test should not crash. (See the bug 39989).
+
+bold first:
+<SPAN contentEditable="">
+<B>
+<#text><selection-anchor>hello, <selection-focus></#text>
+</B>
+<#text>world WebKit</#text>
+</SPAN>
+
+bold middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<B>
+<#text><selection-anchor>world<selection-focus></#text>
+</B>
+<#text> WebKit</#text>
+</SPAN>
+
+bold last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<B>
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</B>
+</SPAN>
+
+bold all:
+<SPAN contentEditable="">
+<B>
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</B>
+</SPAN>
+
+italic first:
+<SPAN contentEditable="">
+<I>
+<#text><selection-anchor>hello, <selection-focus></#text>
+</I>
+<#text>world WebKit</#text>
+</SPAN>
+
+italic middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<I>
+<#text><selection-anchor>world<selection-focus></#text>
+</I>
+<#text> WebKit</#text>
+</SPAN>
+
+italic last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<I>
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</I>
+</SPAN>
+
+italic all:
+<SPAN contentEditable="">
+<I>
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</I>
+</SPAN>
+
+underline first:
+<SPAN contentEditable="">
+<U>
+<#text><selection-anchor>hello, <selection-focus></#text>
+</U>
+<#text>world WebKit</#text>
+</SPAN>
+
+underline middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<U>
+<#text><selection-anchor>world<selection-focus></#text>
+</U>
+<#text> WebKit</#text>
+</SPAN>
+
+underline last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<U>
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</U>
+</SPAN>
+
+underline all:
+<SPAN contentEditable="">
+<U>
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</U>
+</SPAN>
+
+strikeThrough first:
+<SPAN contentEditable="">
+<S>
+<#text><selection-anchor>hello, <selection-focus></#text>
+</S>
+<#text>world WebKit</#text>
+</SPAN>
+
+strikeThrough middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<S>
+<#text><selection-anchor>world<selection-focus></#text>
+</S>
+<#text> WebKit</#text>
+</SPAN>
+
+strikeThrough last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<S>
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</S>
+</SPAN>
+
+strikeThrough all:
+<SPAN contentEditable="">
+<S>
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</S>
+</SPAN>
+
+foreColor first:
+<SPAN contentEditable="">
+<FONT class="Apple-style-span" color="#0000FF">
+<#text><selection-anchor>hello, <selection-focus></#text>
+</FONT>
+<#text>world WebKit</#text>
+</SPAN>
+
+foreColor middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<FONT class="Apple-style-span" color="#0000FF">
+<#text><selection-anchor>world<selection-focus></#text>
+</FONT>
+<#text> WebKit</#text>
+</SPAN>
+
+foreColor last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<FONT class="Apple-style-span" color="#0000FF">
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</FONT>
+</SPAN>
+
+foreColor all:
+<SPAN contentEditable="">
+<FONT class="Apple-style-span" color="#0000FF">
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</FONT>
+</SPAN>
+
+hiliteColor first:
+<SPAN contentEditable="">
+<SPAN style="background-color: blue;" class="Apple-style-span">
+<#text><selection-anchor>hello, <selection-focus></#text>
+</SPAN>
+<#text>world WebKit</#text>
+</SPAN>
+
+hiliteColor middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<SPAN style="background-color: blue;" class="Apple-style-span">
+<#text><selection-anchor>world<selection-focus></#text>
+</SPAN>
+<#text> WebKit</#text>
+</SPAN>
+
+hiliteColor last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<SPAN style="background-color: blue;" class="Apple-style-span">
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</SPAN>
+</SPAN>
+
+hiliteColor all:
+<SPAN contentEditable="">
+<SPAN style="background-color: blue;" class="Apple-style-span">
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</SPAN>
+</SPAN>
+
+subscript first:
+<SPAN contentEditable="">
+<SUB>
+<#text><selection-anchor>hello, <selection-focus></#text>
+</SUB>
+<#text>world WebKit</#text>
+</SPAN>
+
+subscript middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<SUB>
+<#text><selection-anchor>world<selection-focus></#text>
+</SUB>
+<#text> WebKit</#text>
+</SPAN>
+
+subscript last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<SUB>
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</SUB>
+</SPAN>
+
+subscript all:
+<SPAN contentEditable="">
+<SUB>
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</SUB>
+</SPAN>
+
+superscript first:
+<SPAN contentEditable="">
+<SUP>
+<#text><selection-anchor>hello, <selection-focus></#text>
+</SUP>
+<#text>world WebKit</#text>
+</SPAN>
+
+superscript middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<SUP>
+<#text><selection-anchor>world<selection-focus></#text>
+</SUP>
+<#text> WebKit</#text>
+</SPAN>
+
+superscript last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<SUP>
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</SUP>
+</SPAN>
+
+superscript all:
+<SPAN contentEditable="">
+<SUP>
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</SUP>
+</SPAN>
+
+createLink first:
+<SPAN contentEditable="">
+<A href="http://webkit.org/">
+<#text><selection-anchor>hello, <selection-focus></#text>
+</A>
+<#text>world WebKit</#text>
+</SPAN>
+
+createLink middle:
+<SPAN contentEditable="">
+<#text>hello, </#text>
+<A href="http://webkit.org/">
+<#text><selection-anchor>world<selection-focus></#text>
+</A>
+<#text> WebKit</#text>
+</SPAN>
+
+createLink last:
+<SPAN contentEditable="">
+<#text>hello, world</#text>
+<A href="http://webkit.org/">
+<#text><selection-anchor> WebKit<selection-focus></#text>
+</A>
+</SPAN>
+
+createLink all:
+<SPAN contentEditable="">
+<A href="http://webkit.org/">
+<#text><selection-anchor>hello, world WebKit<selection-focus></#text>
+</A>
+</SPAN>
diff --git a/LayoutTests/editing/style/style-text-node-without-editable-parent.html b/LayoutTests/editing/style/style-text-node-without-editable-parent.html
new file mode 100644 (file)
index 0000000..d2f4b5a
--- /dev/null
@@ -0,0 +1,61 @@
+<script src="../../resources/dump-as-markup.js"></script>
+<body>
+<div id="test"></div>
+<script>
+
+function appendNewSpan(title)
+{
+    var div = document.getElementById('test');
+    if (div.innerHTML.length)
+        div.innerHTML += '<br>';
+    div.innerHTML += title + ' : <span contenteditable>hello, world WebKit</span>';
+    return div.lastChild;
+}
+
+function setSelection(text, start, end)
+{
+    var range = document.createRange();
+    var selection = window.getSelection();
+
+    range.setStart(text, start);
+    range.setEnd(text, end);
+    selection.removeAllRanges();
+    selection.addRange(range);
+}
+
+function dumpLastSpan(description) {
+    Markup.dump(document.getElementById('test').lastChild, description)
+}
+
+function applyInlineStyleToTextNode(command, value)
+{
+    setSelection(appendNewSpan(command).firstChild, 0, 7);
+    document.execCommand(command, false, value);
+    dumpLastSpan(command + ' first');
+
+    setSelection(appendNewSpan(command).firstChild, 7, 12);
+    document.execCommand(command, false, value);
+    dumpLastSpan(command + ' middle');
+
+    setSelection(appendNewSpan(command).firstChild, 12, 19);
+    document.execCommand(command, false, value);
+    dumpLastSpan(command + ' last');
+
+    setSelection(appendNewSpan(command).firstChild, 0, 19);
+    document.execCommand(command, false, value);
+    dumpLastSpan(command + ' all');
+}
+
+Markup.description("This tests applying inline style to a text node, which is a child of the editable root. Style should be applied properly and the test should not crash. (See the bug 39989).")
+applyInlineStyleToTextNode('bold');
+applyInlineStyleToTextNode('italic');
+applyInlineStyleToTextNode('underline');
+applyInlineStyleToTextNode('strikeThrough');
+applyInlineStyleToTextNode('foreColor', 'blue');
+applyInlineStyleToTextNode('hiliteColor', 'blue');
+applyInlineStyleToTextNode('subscript');
+applyInlineStyleToTextNode('superscript');
+applyInlineStyleToTextNode('createLink', 'http://webkit.org/');
+
+</script>
+</body>
index 916faa2..eeafce8 100644 (file)
@@ -161,7 +161,7 @@ Markup._spaces = function(multiplier)
 }
 
 // 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)
 {
index 10f6210..bde677a 100644 (file)
@@ -1,3 +1,26 @@
+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>
 
         Reviewed by Anders Carlsson.
index 8e6ce63..8847714 100644 (file)
@@ -1554,6 +1554,10 @@ void ApplyStyleCommand::splitTextAtEnd(const Position& start, const Position& en
 
 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());
     splitTextNodeContainingElement(text, start.deprecatedEditingOffset());
@@ -1562,6 +1566,10 @@ void ApplyStyleCommand::splitTextElementAtStart(const Position& start, const Pos
 
 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());
 
index e716c72..8c90fb0 100644 (file)
@@ -48,7 +48,7 @@ void SplitTextNodeContainingElementCommand::doApply()
     splitTextNode(m_text.get(), m_offset);
 
     Element* parent = m_text->parentElement();
-    if (!parent)
+    if (!parent || !parent->parentElement() || !parent->parentElement()->isContentEditable())
         return;
 
     RenderObject* parentRenderer = parent->renderer();