[webkit-changes] [144995] trunk

Wed, 06 Mar 2013 16:05:03 -0800

Title: [144995] trunk
Revision
144995
Author
[email protected]
Date
2013-03-06 16:07:02 -0800 (Wed, 06 Mar 2013)

Log Message

InsertUnorderedList can lead to lost content and assertions in moveParagraphs
https://bugs.webkit.org/show_bug.cgi?id=111228

Reviewed by Ryosuke Niwa.

Source/WebCore:

When a list is wrapped in a presentational inline like a b tag, we'd create markup that included
everything up to the b tag from moveParagraphs when intending to only move the contents of the
list item. This could make it impossible to remove content from a list and trigger loss of
editable text.

Test: editing/execCommand/insert-remove-block-list-inside-presentational-inline.html

* editing/EditingStyle.cpp:
(WebCore::elementMatchesAndPropertyIsNotInInlineStyleDecl): Ensure there's an inline style
before calling propertyExistsInStyle.
(WebCore::HTMLElementEquivalent::propertyExistsInStyle): Removing null check for style.
All callers ensure this value isn't null.
* editing/markup.cpp:
(WebCore::highestAncestorToWrapMarkup): Avoid going over RenderBlocks when finding the highest
presentational ancestor to avoid leaving the bounds of the original paragraph.

LayoutTests:

* editing/deleting/pruning-after-merge-1-expected.txt:
* editing/execCommand/insert-remove-block-list-inside-presentational-inline-expected.txt: Added.
* editing/execCommand/insert-remove-block-list-inside-presentational-inline.html: Added.
* editing/pasteboard/paste-and-sanitize-expected.txt:
* editing/pasteboard/paste-and-sanitize.html:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (144994 => 144995)


--- trunk/LayoutTests/ChangeLog	2013-03-06 23:56:27 UTC (rev 144994)
+++ trunk/LayoutTests/ChangeLog	2013-03-07 00:07:02 UTC (rev 144995)
@@ -1,3 +1,16 @@
+2013-03-06  Levi Weintraub  <[email protected]>
+
+        InsertUnorderedList can lead to lost content and assertions in moveParagraphs
+        https://bugs.webkit.org/show_bug.cgi?id=111228
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/deleting/pruning-after-merge-1-expected.txt:
+        * editing/execCommand/insert-remove-block-list-inside-presentational-inline-expected.txt: Added.
+        * editing/execCommand/insert-remove-block-list-inside-presentational-inline.html: Added.
+        * editing/pasteboard/paste-and-sanitize-expected.txt:
+        * editing/pasteboard/paste-and-sanitize.html:
+
 2013-03-06  Rafael Weinstein  <[email protected]>
 
         Unreviewed gardening.

Modified: trunk/LayoutTests/editing/deleting/pruning-after-merge-1-expected.txt (144994 => 144995)


--- trunk/LayoutTests/editing/deleting/pruning-after-merge-1-expected.txt	2013-03-06 23:56:27 UTC (rev 144994)
+++ trunk/LayoutTests/editing/deleting/pruning-after-merge-1-expected.txt	2013-03-07 00:07:02 UTC (rev 144995)
@@ -5,7 +5,7 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 1 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > B > DIV > BODY > HTML > #document
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV > B > DIV > BODY > HTML > #document to 2 of #text > DIV > B > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > SPAN > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 This tests the pruning that delete does when the it merges two paragraphs when the selection to delete spans multiple blocks.

Added: trunk/LayoutTests/editing/execCommand/insert-remove-block-list-inside-presentational-inline-expected.txt (0 => 144995)


--- trunk/LayoutTests/editing/execCommand/insert-remove-block-list-inside-presentational-inline-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/insert-remove-block-list-inside-presentational-inline-expected.txt	2013-03-07 00:07:02 UTC (rev 144995)
@@ -0,0 +1,10 @@
+This test verifies insertUnorderedList can properly undo its own DOM manipulation.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS document.getElementsByTagName('ul').length == 0 is true
+

Added: trunk/LayoutTests/editing/execCommand/insert-remove-block-list-inside-presentational-inline.html (0 => 144995)


--- trunk/LayoutTests/editing/execCommand/insert-remove-block-list-inside-presentational-inline.html	                        (rev 0)
+++ trunk/LayoutTests/editing/execCommand/insert-remove-block-list-inside-presentational-inline.html	2013-03-07 00:07:02 UTC (rev 144995)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body _onload_="runTest();">
+<div id="console"></div>
+<div contenteditable="true" id="root"><b>one</b><div><b>two</b></div><div><b>three</b></div><div><br></div></div>
+<script>
+var root = document.getElementById("root");
+function toggleList() {
+    var temporaryBlock = document.createElement('div');
+    temporaryBlock.style.cssText =  "height: 0";
+    temporaryBlock.appendChild(document.createTextNode('x'));
+    root.insertBefore(temporaryBlock, root.firstChild);
+    document.execCommand('insertUnorderedList');
+    root.removeChild(temporaryBlock);
+}
+
+function runTest() {
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    description("This test verifies insertUnorderedList can properly undo its own DOM manipulation.");
+    root.focus();
+    document.execCommand("SelectAll");
+    toggleList();
+    toggleList();
+    toggleList();
+    toggleList();
+    shouldBeTrue("document.getElementsByTagName('ul').length == 0");
+
+    if (window.testRunner)
+        root.parentNode.removeChild(root);
+}
+</script>
+<script src=""
+</body>

Modified: trunk/LayoutTests/editing/pasteboard/paste-and-sanitize-expected.txt (144994 => 144995)


--- trunk/LayoutTests/editing/pasteboard/paste-and-sanitize-expected.txt	2013-03-06 23:56:27 UTC (rev 144994)
+++ trunk/LayoutTests/editing/pasteboard/paste-and-sanitize-expected.txt	2013-03-07 00:07:02 UTC (rev 144995)
@@ -4,7 +4,7 @@
 PASS confirmedMarkup is '<b><i>Hello</i></b>'
 PASS confirmedMarkup is '<b><i>Hello</i></b>'
 PASS confirmedMarkup is 'Hello'
-PASS confirmedMarkup is '<b><i>Hello</i></b>'
+PASS confirmedMarkup is '<i style="font-weight: bold;">Hello</i>'
 PASS confirmedMarkup is '<div style="text-align: center;"><b>Hello</b></div>'
 PASS confirmedMarkup is '<div><b><i>hello</i></b></div><div><b><i>world</i></b></div>'
 PASS confirmedMarkup is '<b><i><span style="font-weight: normal;"><b><i>hello1</i></b><b><i>&nbsp;hello2</i></b></span></i></b>'

Modified: trunk/LayoutTests/editing/pasteboard/paste-and-sanitize.html (144994 => 144995)


--- trunk/LayoutTests/editing/pasteboard/paste-and-sanitize.html	2013-03-06 23:56:27 UTC (rev 144994)
+++ trunk/LayoutTests/editing/pasteboard/paste-and-sanitize.html	2013-03-07 00:07:02 UTC (rev 144995)
@@ -38,7 +38,7 @@
 testPaste("div", "<b><i>Hello</i></b>", "<b><i>Hello</i></b>");
 testPaste("div", "<div><b><i><span style=\"font-weight: normal\"><b><i>Hello</i></b></span></i></b></div>", "<b><i>Hello</i></b>");
 testPaste("div", "<div><div><div>Hello</div></div></div>", "Hello");
-testPaste("div", "<div><b><div><i>Hello</i></div></b></div>", "<b><i>Hello</i></b>");
+testPaste("div", "<div><b><div><i>Hello</i></div></b></div>", "<i style=\"font-weight: bold;\">Hello</i>");
 testPaste("div", "<div><div style=\"text-align: center;\"><b>Hello</b></div></div>", "<div style=\"text-align: center;\"><b>Hello</b></div>");
 testPaste("div", "<div><b><i><span style=\"font-weight: normal\"><b><i>hello</i></b></span></i></b></div><div><b><i><span style=\"font-weight: normal\"><b><i>world</i></b></span></i></b></div>", 
           "<div><b><i>hello</i></b></div><div><b><i>world</i></b></div>");

Modified: trunk/Source/WebCore/ChangeLog (144994 => 144995)


--- trunk/Source/WebCore/ChangeLog	2013-03-06 23:56:27 UTC (rev 144994)
+++ trunk/Source/WebCore/ChangeLog	2013-03-07 00:07:02 UTC (rev 144995)
@@ -1,3 +1,26 @@
+2013-03-06  Levi Weintraub  <[email protected]>
+
+        InsertUnorderedList can lead to lost content and assertions in moveParagraphs
+        https://bugs.webkit.org/show_bug.cgi?id=111228
+
+        Reviewed by Ryosuke Niwa.
+
+        When a list is wrapped in a presentational inline like a b tag, we'd create markup that included
+        everything up to the b tag from moveParagraphs when intending to only move the contents of the
+        list item. This could make it impossible to remove content from a list and trigger loss of
+        editable text.
+
+        Test: editing/execCommand/insert-remove-block-list-inside-presentational-inline.html
+
+        * editing/EditingStyle.cpp:
+        (WebCore::elementMatchesAndPropertyIsNotInInlineStyleDecl): Ensure there's an inline style
+        before calling propertyExistsInStyle.
+        (WebCore::HTMLElementEquivalent::propertyExistsInStyle): Removing null check for style.
+        All callers ensure this value isn't null.
+        * editing/markup.cpp:
+        (WebCore::highestAncestorToWrapMarkup): Avoid going over RenderBlocks when finding the highest
+        presentational ancestor to avoid leaving the bounds of the original paragraph.
+
 2013-03-06  Adam Klein  <[email protected]>
 
         [V8] Use implicit references instead of object groups to keep registered MutationObservers alive

Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (144994 => 144995)


--- trunk/Source/WebCore/editing/EditingStyle.cpp	2013-03-06 23:56:27 UTC (rev 144994)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp	2013-03-07 00:07:02 UTC (rev 144995)
@@ -128,7 +128,7 @@
     virtual ~HTMLElementEquivalent() { }
     virtual bool matches(const Element* element) const { return !m_tagName || element->hasTagName(*m_tagName); }
     virtual bool hasAttribute() const { return false; }
-    virtual bool propertyExistsInStyle(const StylePropertySet* style) const { return style && style->getPropertyCSSValue(m_propertyID); }
+    virtual bool propertyExistsInStyle(const StylePropertySet* style) const { return style->getPropertyCSSValue(m_propertyID); }
     virtual bool valueIsPresentInStyle(Element*, StylePropertySet*) const;
     virtual void addToStyle(Element*, EditingStyle*) const;
 
@@ -974,7 +974,7 @@
 static inline bool elementMatchesAndPropertyIsNotInInlineStyleDecl(const HTMLElementEquivalent* equivalent, const StyledElement* element,
     EditingStyle::CSSPropertyOverrideMode mode, StylePropertySet* style)
 {
-    return equivalent->matches(element) && !equivalent->propertyExistsInStyle(element->inlineStyle())
+    return equivalent->matches(element) && (!element->inlineStyle() || !equivalent->propertyExistsInStyle(element->inlineStyle()))
         && (mode == EditingStyle::OverrideValues || !equivalent->propertyExistsInStyle(style));
 }

Modified: trunk/Source/WebCore/editing/markup.cpp (144994 => 144995)


--- trunk/Source/WebCore/editing/markup.cpp	2013-03-06 23:56:27 UTC (rev 144994)
+++ trunk/Source/WebCore/editing/markup.cpp	2013-03-07 00:07:02 UTC (rev 144995)
@@ -56,6 +56,7 @@
 #include "MarkupAccumulator.h"
 #include "NodeTraversal.h"
 #include "Range.h"
+#include "RenderBlock.h"
 #include "RenderObject.h"
 #include "Settings.h"
 #include "StylePropertySet.h"
@@ -522,7 +523,7 @@
 
     Node* checkAncestor = specialCommonAncestor ? specialCommonAncestor : commonAncestor;
     if (checkAncestor->renderer()) {
-        Node* newSpecialCommonAncestor = highestEnclosingNodeOfType(firstPositionInNode(checkAncestor), &isElementPresentational);
+        Node* newSpecialCommonAncestor = highestEnclosingNodeOfType(firstPositionInNode(checkAncestor), &isElementPresentational, CanCrossEditingBoundary, checkAncestor->renderer()->containingBlock()->node());
         if (newSpecialCommonAncestor)
             specialCommonAncestor = newSpecialCommonAncestor;
     }

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to