Commit 94f8b8fd authored by leviw@chromium.org's avatar leviw@chromium.org
Browse files

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:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@144995 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 38107efd
2013-03-06 Levi Weintraub <leviw@chromium.org>
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 <rafaelw@chromium.org>
 
Unreviewed gardening.
......@@ -5,7 +5,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
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.
......
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
<!DOCTYPE html>
<html>
<head>
<script src="../../fast/js/resources/js-test-pre.js"></script>
</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="../../fast/js/resources/js-test-post.js"></script>
</body>
......@@ -4,7 +4,7 @@ PASS confirmedMarkup is 'Hello'
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>'
......
......@@ -38,7 +38,7 @@ testPaste("div", "Hello", "Hello");
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>");
......
2013-03-06 Levi Weintraub <leviw@chromium.org>
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 <adamk@chromium.org>
 
[V8] Use implicit references instead of object groups to keep registered MutationObservers alive
......@@ -128,7 +128,7 @@ public:
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 @@ void EditingStyle::mergeInlineStyleOfElement(StyledElement* element, CSSProperty
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));
}
......
......@@ -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 @@ static Node* highestAncestorToWrapMarkup(const Range* range, EAnnotateForInterch
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;
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment