[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

rniwa at webkit.org rniwa at webkit.org
Wed Dec 22 12:35:52 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 98fe553ceb3e5257c1d522d4e35944997e10eeef
Author: rniwa at webkit.org <rniwa at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Wed Aug 25 21:15:34 2010 +0000

    2010-08-24  Ryosuke Niwa  <rniwa at webkit.org>
    
            Reviewed by Tony Chang.
    
            Creating a link when selecting multiple nodes creates multiple links
            https://bugs.webkit.org/show_bug.cgi?id=30836
    
            The bug was caused by applyInlineStyleToRange calling addInlineStyleIfNeeded
            on each inline element. Modified applyInlineStyleToRange to call addInlineStyleIfNeeded
            once for all inline elements with the same style difference.
    
            Because this implies that anchor element may wrap other inline elements when added,
            modified pushDownInlineStyleAroundNode to push down styled elements.
    
            Removed pushPartiallySelectedAnchorElementsDown from CompositeEditCommand since
            ApplyStyleCommand now correctly pushes down anchors at the start and the end of the selection.
    
            Test: editing/execCommand/toggle-link.html
    
            * editing/ApplyStyleCommand.cpp:
            (WebCore::StyleChange::operator==): Added.
            (WebCore::StyleChange::operator!=): Added.
            (WebCore::ApplyStyleCommand::applyInlineStyleToRange): Wraps inline elements with
            the same style difference by one element instead of wrapping each element separately.
            (WebCore::ApplyStyleCommand::extractInlineStyleToPushDown): Extracts styled element.
            (WebCore::ApplyStyleCommand::applyInlineStyleToPushDown): Avoids adding styled element.
            (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): Pushes down styled element.
            (WebCore::ApplyStyleCommand::surroundNodeRangeWithElement): No longer checks inline-ness.
            (WebCore::ApplyStyleCommand::addInlineStyleIfNeeded): Adds style even when m_removeOnly.
            Callers should set addStyledElement = DoNotAddStyledElement to avoid adding styled element.
            * editing/ApplyStyleCommand.h:
            * editing/CompositeEditCommand.cpp: Removed pushPartiallySelectedAnchorElementsDown.
            * editing/CompositeEditCommand.h: Removed pushPartiallySelectedAnchorElementsDown.
            * editing/CreateLinkCommand.cpp:
            (WebCore::CreateLinkCommand::doApply): used to call pushPartiallySelectedAnchorElementsDown.
            * editing/UnlinkCommand.cpp:
            (WebCore::UnlinkCommand::doApply): Used to call pushPartiallySelectedAnchorElementsDown.
    2010-08-24  Ryosuke Niwa  <rniwa at webkit.org>
    
            Reviewed by Tony Chang.
    
            Creating a link when selecting multiple nodes creates multiple links
            https://bugs.webkit.org/show_bug.cgi?id=30836
    
            Added a test to ensure WebKit creates single anchor element on execCommand('createLink') if possible.
            Also rebaselined several tests to match new behavior.
    
            * editing/execCommand/createLink-expected.txt: Merged some anchor elements.
            * editing/execCommand/script-tests/toggle-link.js: Added.
            (testSingleToggle):
            (selectAll):
            (selectFirstTwoWords):
            (selectLastWord):
            * editing/execCommand/script-tests/toggle-style-3.js: i elements are merged.
            * editing/execCommand/toggle-style-3-expected.txt: Ditto.
            * editing/execCommand/toggle-link-expected.txt: Added.
            * editing/execCommand/toggle-link.html: Added.
            * editing/execCommand/unlink-expected.txt: Merged some anchor elements.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66040 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 2aeb1ce..9a11455 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,25 @@
+2010-08-24  Ryosuke Niwa  <rniwa at webkit.org>
+
+        Reviewed by Tony Chang.
+
+        Creating a link when selecting multiple nodes creates multiple links
+        https://bugs.webkit.org/show_bug.cgi?id=30836
+
+        Added a test to ensure WebKit creates single anchor element on execCommand('createLink') if possible.
+        Also rebaselined several tests to match new behavior.
+
+        * editing/execCommand/createLink-expected.txt: Merged some anchor elements.
+        * editing/execCommand/script-tests/toggle-link.js: Added.
+        (testSingleToggle):
+        (selectAll):
+        (selectFirstTwoWords):
+        (selectLastWord):
+        * editing/execCommand/script-tests/toggle-style-3.js: i elements are merged.
+        * editing/execCommand/toggle-style-3-expected.txt: Ditto.
+        * editing/execCommand/toggle-link-expected.txt: Added.
+        * editing/execCommand/toggle-link.html: Added.
+        * editing/execCommand/unlink-expected.txt: Merged some anchor elements.
+
 2010-08-24  Zhenyao Mo  <zmo at google.com>
 
         Reviewed by Kenneth Russell.
diff --git a/LayoutTests/editing/execCommand/createLink-expected.txt b/LayoutTests/editing/execCommand/createLink-expected.txt
index 4d294ae..d0fe963 100644
--- a/LayoutTests/editing/execCommand/createLink-expected.txt
+++ b/LayoutTests/editing/execCommand/createLink-expected.txt
@@ -13,8 +13,7 @@ EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 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 > A > DIV > LI > OL > BODY > HTML > #document to 10 of #text > A > DIV > LI > OL > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 4 of #text > A > DIV > LI > OL > BODY > HTML > #document to 14 of #text > A > DIV > LI > OL > BODY > HTML > #document toDOMRange:range from 0 of #text > A > DIV > LI > OL > BODY > HTML > #document to 10 of #text > A > DIV > LI > OL > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of DIV > LI > OL > BODY > HTML > #document to 3 of DIV > LI > OL > BODY > HTML > #document
@@ -59,4 +58,4 @@ innerHTML of editable regions after the test:
 <a href="http://www.google.com/">This paragraph starts unlinked. The entire paragraph should end up being a link to google.com.</a>
 <a href="http://www.apple.com/">This</a><a href="http://www.google.com/"> paragraph</a><a href="http://www.apple.com/"> starts out as a link to apple.com. The second word in the paragraph should end up being a link to google.com.</a>
 <a href="http://www.apple.com/">This</a><a href="http://www.google.com/"> paragraph</a><a href="http://www.apple.com/"> starts out as a link to apple.com. The second word in the paragraph should end up being a link to google.com.</a>
-<p><a href="http://www.google.com/">This </a><i><a href="http://www.google.com/">editable region</a></i><a href="http://www.google.com/"> contains lists, tables, styled text, and images. The entire region should end up being a link to google.com.</a></p> <ul> <li><a href="http://www.google.com/">Item 1</a></li> <li><a href="http://www.google.com/">Item 2</a></li> </ul> <table border="1"><tbody><tr><td><a href="http://www.google.com/">1</a></td><td><a href="http://www.google.com/">2</a></td><td><a href="http://www.google.com/">3</a></td></tr></tbody></table> <a href="http://www.google.com/"><br> This </a><b><a href="http://www.google.com/">line</a></b><a href="http://www.google.com/"> contains <img src="../resources/abe.png"> an image. </a>
+<p><a href="http://www.google.com/">This <i>editable region</i> contains lists, tables, styled text, and images. The entire region should end up being a link to google.com.</a></p> <ul> <li><a href="http://www.google.com/">Item 1</a></li> <li><a href="http://www.google.com/">Item 2</a></li> </ul> <table border="1"><tbody><tr><td><a href="http://www.google.com/">1</a></td><td><a href="http://www.google.com/">2</a></td><td><a href="http://www.google.com/">3</a></td></tr></tbody></table> <a href="http://www.google.com/"><br> This <b>line</b> contains <img src="../resources/abe.png"> an image. </a>
diff --git a/LayoutTests/editing/execCommand/script-tests/toggle-link.js b/LayoutTests/editing/execCommand/script-tests/toggle-link.js
new file mode 100644
index 0000000..43c924c
--- /dev/null
+++ b/LayoutTests/editing/execCommand/script-tests/toggle-link.js
@@ -0,0 +1,54 @@
+description("Test to make sure we remove span tags with no attributes if we removed the last attribute.")
+
+var testContainer = document.createElement("div");
+testContainer.contentEditable = true;
+document.body.appendChild(testContainer);
+
+function testSingleToggle(toggleCommand, initialContents, selector, expectedContents)
+{
+    testContainer.innerHTML = initialContents;
+    var selected = selector(testContainer);
+    document.execCommand(toggleCommand, false, 'http://webkit.org/');
+    action = 'select ' + selected + ' of "' + initialContents + '" and ' + toggleCommand + ' (http://webkit.org/) yields "' + testContainer.innerHTML + '"';
+    if (testContainer.innerHTML === expectedContents)
+        testPassed(action);
+    else
+        testFailed(action + ', expected "' + expectedContents + '"');
+}
+
+function selectAll(container) {
+    window.getSelection().selectAllChildren(container);
+    return 'all';
+}
+
+function selectFirstTwoWords(container) {
+    window.getSelection().setPosition(container, 0);
+    window.getSelection().modify('extend', 'forward', 'word');
+    window.getSelection().modify('extend', 'forward', 'word');
+    return 'first two words';
+}
+
+function selectLastWord(container) {
+    window.getSelection().setPosition(container, container.childNodes.length);
+    window.getSelection().modify('extend', 'backward', 'word');
+    return 'last word';
+}
+
+testSingleToggle("createLink", 'hello <b>world</b>', selectAll, '<a href="http://webkit.org/">hello <b>world</b></a>');
+testSingleToggle("createLink", '<u>hello world</u>', selectAll, '<u><a href="http://webkit.org/">hello world</a></u>');
+testSingleToggle("createLink", 'hello <a href="http://bugs.webkit.org/">world</a>', selectAll, '<a href="http://webkit.org/">hello world</a>');
+testSingleToggle("createLink", 'hello <a href="http://bugs.webkit.org/" style="font-weight: bold">world</a>', selectAll, '<a href="http://webkit.org/">hello <b>world</b></a>');
+testSingleToggle("createLink", 'hello <b>world</b> WebKit', selectFirstTwoWords, '<a href="http://webkit.org/">hello <b>world</b></a> WebKit');
+testSingleToggle("createLink", '<a href="http://trac.webkit.org/">hello <b>world</b></a> WebKit', selectFirstTwoWords, '<a href="http://webkit.org/">hello <b>world</b></a> WebKit');
+testSingleToggle("createLink", '<a href="http://trac.webkit.org/" style="font-style: italic;">hello world</a> WebKit', selectFirstTwoWords, '<i><a href="http://webkit.org/">hello world</a></i> WebKit');
+testSingleToggle("createLink", 'hello <a href="http://trac.webkit.org/"><b>world</b> WebKit</a>', selectFirstTwoWords, '<a href="http://webkit.org/">hello <b>world</b></a><a href="http://trac.webkit.org/"> WebKit</a>');
+testSingleToggle("createLink", 'hello <a href="http://trac.webkit.org/" style="font-style: italic;"><b>world</b> WebKit</a>', selectFirstTwoWords, '<a href="http://webkit.org/">hello <b style="font-style: italic; ">world</b></a><a href="http://trac.webkit.org/"><i> WebKit</i></a>');
+testSingleToggle("createLink", 'hello <b>world</b> WebKit', selectLastWord, 'hello <b>world</b> <a href="http://webkit.org/">WebKit</a>');
+testSingleToggle("createLink", '<u>hello <b>world</b> WebKit</u>', selectLastWord, '<u>hello <b>world</b> <a href="http://webkit.org/">WebKit</a></u>');
+testSingleToggle("createLink", '<a href="http://trac.webkit.org/"><div>hello</div><div>world</div></a>', selectLastWord, '<div><a href="http://trac.webkit.org/">hello</a></div><div><a href="http://webkit.org/">world</a></div>');
+testSingleToggle("createLink", '<a href="http://trac.webkit.org/" style="font-weight: bold;"><div>hello</div><div>world</div></a>', selectLastWord, '<div style="font-weight: bold; "><a href="http://trac.webkit.org/">hello</a></div><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>');
+testSingleToggle("createLink", '<a href="http://trac.webkit.org/" style="font-weight: bold;"><div style="font-weight: normal;">hello</div><div>world</div></a>', selectLastWord, '<div style="font-weight: normal; "><a href="http://trac.webkit.org/">hello</a></div><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>');
+
+document.body.removeChild(testContainer);
+
+var successfullyParsed = true;
diff --git a/LayoutTests/editing/execCommand/script-tests/toggle-style-3.js b/LayoutTests/editing/execCommand/script-tests/toggle-style-3.js
index f4c8b83..6d4ffb9 100644
--- a/LayoutTests/editing/execCommand/script-tests/toggle-style-3.js
+++ b/LayoutTests/editing/execCommand/script-tests/toggle-style-3.js
@@ -18,7 +18,7 @@ function testSingleToggle(toggleCommand, initialContents, expectedContents)
 
 testSingleToggle("bold", 'hello<b id="test">world</b>', '<b>hello</b><b id="test">world</b>');
 testSingleToggle("bold", 'hello<b><i>world</i></b>', '<b>hello<i>world</i></b>');
-testSingleToggle("italic", 'hello <i>world</i> <b>webkit</b>', '<i>hello world </i><b><i>webkit</i></b>');
+testSingleToggle("italic", 'hello <i>world</i> <b>webkit</b>', '<i>hello world <b>webkit</b></i>');
 testSingleToggle("italic", 'hello <i>world</i> webkit', '<i>hello world webkit</i>');
 
 document.body.removeChild(testContainer);
diff --git a/LayoutTests/editing/execCommand/toggle-link-expected.txt b/LayoutTests/editing/execCommand/toggle-link-expected.txt
new file mode 100644
index 0000000..f88935f
--- /dev/null
+++ b/LayoutTests/editing/execCommand/toggle-link-expected.txt
@@ -0,0 +1,23 @@
+Test to make sure we remove span tags with no attributes if we removed the last attribute.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS select all of "hello <b>world</b>" and createLink (http://webkit.org/) yields "<a href="http://webkit.org/">hello <b>world</b></a>"
+PASS select all of "<u>hello world</u>" and createLink (http://webkit.org/) yields "<u><a href="http://webkit.org/">hello world</a></u>"
+PASS select all of "hello <a href="http://bugs.webkit.org/">world</a>" and createLink (http://webkit.org/) yields "<a href="http://webkit.org/">hello world</a>"
+PASS select all of "hello <a href="http://bugs.webkit.org/" style="font-weight: bold">world</a>" and createLink (http://webkit.org/) yields "<a href="http://webkit.org/">hello <b>world</b></a>"
+PASS select first two words of "hello <b>world</b> WebKit" and createLink (http://webkit.org/) yields "<a href="http://webkit.org/">hello <b>world</b></a> WebKit"
+PASS select first two words of "<a href="http://trac.webkit.org/">hello <b>world</b></a> WebKit" and createLink (http://webkit.org/) yields "<a href="http://webkit.org/">hello <b>world</b></a> WebKit"
+PASS select first two words of "<a href="http://trac.webkit.org/" style="font-style: italic;">hello world</a> WebKit" and createLink (http://webkit.org/) yields "<i><a href="http://webkit.org/">hello world</a></i> WebKit"
+PASS select first two words of "hello <a href="http://trac.webkit.org/"><b>world</b> WebKit</a>" and createLink (http://webkit.org/) yields "<a href="http://webkit.org/">hello <b>world</b></a><a href="http://trac.webkit.org/"> WebKit</a>"
+PASS select first two words of "hello <a href="http://trac.webkit.org/" style="font-style: italic;"><b>world</b> WebKit</a>" and createLink (http://webkit.org/) yields "<a href="http://webkit.org/">hello <b style="font-style: italic; ">world</b></a><a href="http://trac.webkit.org/"><i> WebKit</i></a>"
+PASS select last word of "hello <b>world</b> WebKit" and createLink (http://webkit.org/) yields "hello <b>world</b> <a href="http://webkit.org/">WebKit</a>"
+PASS select last word of "<u>hello <b>world</b> WebKit</u>" and createLink (http://webkit.org/) yields "<u>hello <b>world</b> <a href="http://webkit.org/">WebKit</a></u>"
+PASS select last word of "<a href="http://trac.webkit.org/"><div>hello</div><div>world</div></a>" and createLink (http://webkit.org/) yields "<div><a href="http://trac.webkit.org/">hello</a></div><div><a href="http://webkit.org/">world</a></div>"
+PASS select last word of "<a href="http://trac.webkit.org/" style="font-weight: bold;"><div>hello</div><div>world</div></a>" and createLink (http://webkit.org/) yields "<div style="font-weight: bold; "><a href="http://trac.webkit.org/">hello</a></div><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>"
+PASS select last word of "<a href="http://trac.webkit.org/" style="font-weight: bold;"><div style="font-weight: normal;">hello</div><div>world</div></a>" and createLink (http://webkit.org/) yields "<div style="font-weight: normal; "><a href="http://trac.webkit.org/">hello</a></div><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/execCommand/toggle-link.html b/LayoutTests/editing/execCommand/toggle-link.html
new file mode 100644
index 0000000..f81cdec
--- /dev/null
+++ b/LayoutTests/editing/execCommand/toggle-link.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/toggle-link.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/editing/execCommand/toggle-style-3-expected.txt b/LayoutTests/editing/execCommand/toggle-style-3-expected.txt
index e7efc39..f20af7b 100644
--- a/LayoutTests/editing/execCommand/toggle-style-3-expected.txt
+++ b/LayoutTests/editing/execCommand/toggle-style-3-expected.txt
@@ -5,7 +5,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS one bold command converted hello<b id="test">world</b> to <b>hello</b><b id="test">world</b>
 PASS one bold command converted hello<b><i>world</i></b> to <b>hello<i>world</i></b>
-PASS one italic command converted hello <i>world</i> <b>webkit</b> to <i>hello world </i><b><i>webkit</i></b>
+PASS one italic command converted hello <i>world</i> <b>webkit</b> to <i>hello world <b>webkit</b></i>
 PASS one italic command converted hello <i>world</i> webkit to <i>hello world webkit</i>
 PASS successfullyParsed is true
 
diff --git a/LayoutTests/editing/execCommand/unlink-expected.txt b/LayoutTests/editing/execCommand/unlink-expected.txt
index 7df305c..5e04322 100644
--- a/LayoutTests/editing/execCommand/unlink-expected.txt
+++ b/LayoutTests/editing/execCommand/unlink-expected.txt
@@ -13,8 +13,7 @@ EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 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 > LI > OL > BODY > HTML > #document to 7 of #text > DIV > LI > OL > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 3 of #text > A > DIV > LI > OL > BODY > HTML > #document to 10 of #text > A > DIV > LI > OL > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > LI > OL > BODY > HTML > #document to 7 of #text > DIV > LI > OL > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of DIV > LI > OL > BODY > HTML > #document to 3 of DIV > LI > OL > BODY > HTML > #document
@@ -66,4 +65,4 @@ The innerHTML of editable regions after the test:
 This paragraph should should end up unlinked.
 <a href="http://www.apple.com/">The</a> second<a href="http://www.apple.com/"> word in this paragraph should end up being unlinked, everything else should be a link.</a>
 This paragraph starts with <i><a href="http://www.google.com">a</a></i><span id="test3start"> link</span> in the middle. Only the 'a' in the previous sentence should be linked after the test.
-<p>This <i>editable region</i> contains lists, tables, styled text, and images. Everything in this region that is not selected should be a link, nothing that is selected should be a link.</p> <ul> <li>Item 1</li> <li>Item 2</li> </ul> <table border="1"><tbody><tr><td>1</td><td>2</td><td><span id="test4end"><a href="http://www.google.com/">3</a></span></td></tr></tbody></table> <a href="http://www.google.com/"><br> This </a><b><a href="http://www.google.com/">line</a></b><a href="http://www.google.com/"> contains <img src="../resources/abe.png"> an image. </a>
+<p>This <i>editable region</i> contains lists, tables, styled text, and images. Everything in this region that is not selected should be a link, nothing that is selected should be a link.</p> <ul> <li>Item 1</li> <li>Item 2</li> </ul> <table border="1"><tbody><tr><td>1</td><td>2</td><td><span id="test4end"><a href="http://www.google.com/">3</a></span></td></tr></tbody></table> <a href="http://www.google.com/"><br> This <b>line</b> contains <img src="../resources/abe.png"> an image. </a>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index aac132e..8af2012 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,41 @@
+2010-08-24  Ryosuke Niwa  <rniwa at webkit.org>
+
+        Reviewed by Tony Chang.
+
+        Creating a link when selecting multiple nodes creates multiple links
+        https://bugs.webkit.org/show_bug.cgi?id=30836
+
+        The bug was caused by applyInlineStyleToRange calling addInlineStyleIfNeeded
+        on each inline element. Modified applyInlineStyleToRange to call addInlineStyleIfNeeded
+        once for all inline elements with the same style difference.
+
+        Because this implies that anchor element may wrap other inline elements when added,
+        modified pushDownInlineStyleAroundNode to push down styled elements.
+
+        Removed pushPartiallySelectedAnchorElementsDown from CompositeEditCommand since
+        ApplyStyleCommand now correctly pushes down anchors at the start and the end of the selection.
+
+        Test: editing/execCommand/toggle-link.html
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::StyleChange::operator==): Added.
+        (WebCore::StyleChange::operator!=): Added.
+        (WebCore::ApplyStyleCommand::applyInlineStyleToRange): Wraps inline elements with
+        the same style difference by one element instead of wrapping each element separately.
+        (WebCore::ApplyStyleCommand::extractInlineStyleToPushDown): Extracts styled element.
+        (WebCore::ApplyStyleCommand::applyInlineStyleToPushDown): Avoids adding styled element.
+        (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): Pushes down styled element.
+        (WebCore::ApplyStyleCommand::surroundNodeRangeWithElement): No longer checks inline-ness.
+        (WebCore::ApplyStyleCommand::addInlineStyleIfNeeded): Adds style even when m_removeOnly.
+        Callers should set addStyledElement = DoNotAddStyledElement to avoid adding styled element.
+        * editing/ApplyStyleCommand.h:
+        * editing/CompositeEditCommand.cpp: Removed pushPartiallySelectedAnchorElementsDown.
+        * editing/CompositeEditCommand.h: Removed pushPartiallySelectedAnchorElementsDown.
+        * editing/CreateLinkCommand.cpp:
+        (WebCore::CreateLinkCommand::doApply): used to call pushPartiallySelectedAnchorElementsDown.
+        * editing/UnlinkCommand.cpp:
+        (WebCore::UnlinkCommand::doApply): Used to call pushPartiallySelectedAnchorElementsDown.
+
 2010-08-24  Zhenyao Mo  <zmo at google.com>
 
         Reviewed by Kenneth Russell.
diff --git a/WebCore/editing/ApplyStyleCommand.cpp b/WebCore/editing/ApplyStyleCommand.cpp
index bfbfab8..0a02e7a 100644
--- a/WebCore/editing/ApplyStyleCommand.cpp
+++ b/WebCore/editing/ApplyStyleCommand.cpp
@@ -90,6 +90,23 @@ public:
     String fontFace() { return m_applyFontFace; }
     String fontSize() { return m_applyFontSize; }
 
+    bool operator==(const StyleChange& other)
+    {
+        return m_cssStyle == other.m_cssStyle
+            && m_applyBold == other.m_applyBold
+            && m_applyItalic == other.m_applyItalic
+            && m_applyUnderline == other.m_applyUnderline
+            && m_applyLineThrough == other.m_applyLineThrough
+            && m_applySubscript == other.m_applySubscript
+            && m_applySuperscript == other.m_applySuperscript
+            && m_applyFontColor == other.m_applyFontColor
+            && m_applyFontFace == other.m_applyFontFace
+            && m_applyFontSize == other.m_applyFontSize;
+    }
+    bool operator!=(const StyleChange& other)
+    {
+        return !(*this == other);
+    }
 private:
     void init(PassRefPtr<CSSStyleDeclaration>, const Position&);
     void reconcileTextDecorationProperties(CSSMutableStyleDeclaration*);
@@ -1103,14 +1120,17 @@ void ApplyStyleCommand::applyInlineStyleToRange(CSSMutableStyleDeclaration* styl
             Node* runStart = node;
             // Find the end of the run.
             Node* sibling = node->nextSibling();
-            while (sibling && sibling != pastEndNode && (!sibling->isElementNode() || sibling->hasTagName(brTag)) && !isBlock(sibling)) {
+            StyleChange startChange(style, Position(node, 0));
+            while (sibling && sibling != pastEndNode && !sibling->contains(pastEndNode)
+                   && (!isBlock(sibling) || sibling->hasTagName(brTag))
+                   && StyleChange(style, Position(sibling, 0)) == startChange) {
                 node = sibling;
                 sibling = node->nextSibling();
             }
             // Recompute next, since node has changed.
-            next = node->traverseNextNode();
+            next = node->traverseNextSibling();
             // Apply the style to the run.
-            addInlineStyleIfNeeded(style, runStart, node);
+            addInlineStyleIfNeeded(style, runStart, node, m_removeOnly ? DoNotAddStyledElement : AddStyledElement);
         }
     }
 }
@@ -1344,7 +1364,7 @@ HTMLElement* ApplyStyleCommand::highestAncestorWithConflictingInlineStyle(CSSMut
     return result;
 }
 
-PassRefPtr<CSSMutableStyleDeclaration> ApplyStyleCommand::extractInlineStyleToPushDown(Node* node, const Vector<int>& properties)
+PassRefPtr<CSSMutableStyleDeclaration> ApplyStyleCommand::extractInlineStyleToPushDown(Node* node, bool isStyledElement, const Vector<int>& properties)
 {
     ASSERT(node);
     ASSERT(node->isElementNode());
@@ -1353,13 +1373,17 @@ PassRefPtr<CSSMutableStyleDeclaration> ApplyStyleCommand::extractInlineStyleToPu
     if (!node->isHTMLElement())
         return 0;
 
-    HTMLElement *element = static_cast<HTMLElement *>(node);
+    HTMLElement* element = static_cast<HTMLElement*>(node);
     RefPtr<CSSMutableStyleDeclaration> style = element->inlineStyleDecl();
+    if (isStyledElement) {
+        removeNodePreservingChildren(element);
+        return style.release();
+    }
+
     if (!style)
         return 0;
 
     style = style->copyPropertiesInSet(properties.data(), properties.size());
-
     for (size_t i = 0; i < properties.size(); i++) {
         RefPtr<CSSValue> property = style->getPropertyCSSValue(properties[i]);
         if (property)
@@ -1429,7 +1453,10 @@ void ApplyStyleCommand::applyInlineStyleToPushDown(Node* node, CSSMutableStyleDe
         return;
 
     // FIXME: addInlineStyleIfNeeded may override the style of node
-    addInlineStyleIfNeeded(style, node, node);
+    // We can't wrap node with the styled element here because new styled element will never be removed if we did.
+    // If we modified the child pointer in pushDownInlineStyleAroundNode to point to new style element
+    // then we fall into an infinite loop where we keep removing and adding styled element wrapping node.
+    addInlineStyleIfNeeded(style, node, node, DoNotAddStyledElement);
 }
 
 void ApplyStyleCommand::pushDownInlineStyleAroundNode(CSSMutableStyleDeclaration* style, Node* targetNode)
@@ -1450,14 +1477,33 @@ void ApplyStyleCommand::pushDownInlineStyleAroundNode(CSSMutableStyleDeclaration
         ASSERT(current->isHTMLElement());
         ASSERT(current->contains(targetNode));
         Node* child = current->firstChild();
-        RefPtr<CSSMutableStyleDeclaration> styleToPushDown = extractInlineStyleToPushDown(current, properties);
+        Node* lastChild = current->lastChild();
+        RefPtr<StyledElement> styledElement;
+        if (current->isStyledElement() && m_styledInlineElement && current->hasTagName(m_styledInlineElement->tagQName()))
+            styledElement = static_cast<StyledElement*>(current);
+        RefPtr<CSSMutableStyleDeclaration> styleToPushDown = extractInlineStyleToPushDown(current, styledElement, properties);
 
         // The inner loop will go through children on each level
+        // FIXME: we should aggregate inline child elements together so that we don't wrap each child separately.
         while (child) {
             Node* nextChild = child->nextSibling();
 
+            if (child != targetNode && styledElement) {
+                // If child has children, wrap children of child by a clone of the styled element to avoid infinite loop.
+                // Otherwise, wrap the child by the styled element, and we won't fall into an infinite loop.
+                RefPtr<Element> wrapper = styledElement->cloneElementWithoutChildren();
+                ExceptionCode ec = 0;
+                wrapper->removeAttribute(styleAttr, ec);
+                ASSERT(!ec);
+                if (child->firstChild())
+                    surroundNodeRangeWithElement(child->firstChild(), child->lastChild(), wrapper);
+                else
+                    surroundNodeRangeWithElement(child, child, wrapper);
+            }
+
             // Apply text decoration to all nodes containing targetNode and their siblings but NOT to targetNode
-            if (child != targetNode)
+            // But if we've removed styledElement then go ahead and always apply the style.
+            if (child != targetNode || styledElement)
                 applyInlineStyleToPushDown(child, styleToPushDown.get());
 
             // We found the next node for the outer loop (contains targetNode)
@@ -1465,6 +1511,8 @@ void ApplyStyleCommand::pushDownInlineStyleAroundNode(CSSMutableStyleDeclaration
             if (child == targetNode || child->contains(targetNode))
                 current = child;
 
+            if (child == lastChild || child->contains(lastChild))
+                break;
             child = nextChild;
         }
     }
@@ -1737,11 +1785,9 @@ void ApplyStyleCommand::surroundNodeRangeWithElement(Node* startNode, Node* endN
     
     Node* node = startNode;
     while (1) {
-        Node* next = node->traverseNextNode();
-        if (node->childNodeCount() == 0 && node->renderer() && node->renderer()->isInline()) {
-            removeNode(node);
-            appendNode(node, element);
-        }
+        Node* next = node->nextSibling();
+        removeNode(node);
+        appendNode(node, element);
         if (node == endNode)
             break;
         node = next;
@@ -1779,11 +1825,8 @@ void ApplyStyleCommand::addBlockStyle(const StyleChange& styleChange, HTMLElemen
     setNodeAttribute(block, styleAttr, cssText);
 }
 
-void ApplyStyleCommand::addInlineStyleIfNeeded(CSSMutableStyleDeclaration *style, Node *startNode, Node *endNode)
+void ApplyStyleCommand::addInlineStyleIfNeeded(CSSMutableStyleDeclaration *style, Node *startNode, Node *endNode, EAddStyledElement addStyledElement)
 {
-    if (m_removeOnly)
-        return;
-
     StyleChange styleChange(style, Position(startNode, 0));
 
     // Font tags need to go outside of CSS so that CSS font sizes override leagcy font sizes.
@@ -1823,7 +1866,7 @@ void ApplyStyleCommand::addInlineStyleIfNeeded(CSSMutableStyleDeclaration *style
     else if (styleChange.applySuperscript())
         surroundNodeRangeWithElement(startNode, endNode, createHTMLElement(document(), supTag));
 
-    if (m_styledInlineElement)
+    if (m_styledInlineElement && addStyledElement == AddStyledElement)
         surroundNodeRangeWithElement(startNode, endNode, m_styledInlineElement->cloneElementWithoutChildren());
 }
 
diff --git a/WebCore/editing/ApplyStyleCommand.h b/WebCore/editing/ApplyStyleCommand.h
index f4ecc7c..abe4909 100644
--- a/WebCore/editing/ApplyStyleCommand.h
+++ b/WebCore/editing/ApplyStyleCommand.h
@@ -43,6 +43,7 @@ class ApplyStyleCommand : public CompositeEditCommand {
 public:
     enum EPropertyLevel { PropertyDefault, ForceBlockProperties };
     enum InlineStyleRemovalMode { RemoveAttributesAndElements, RemoveNone };
+    enum EAddStyledElement { AddStyledElement, DoNotAddStyledElement };
 
     static PassRefPtr<ApplyStyleCommand> create(Document* document, CSSStyleDeclaration* style, EditAction action = EditActionChangeAttributes, EPropertyLevel level = PropertyDefault)
     {
@@ -79,7 +80,7 @@ private:
     bool removeHTMLBidiEmbeddingStyle(CSSMutableStyleDeclaration*, HTMLElement*, InlineStyleRemovalMode = RemoveAttributesAndElements);
     bool removeCSSStyle(CSSMutableStyleDeclaration*, HTMLElement*, InlineStyleRemovalMode = RemoveAttributesAndElements);
     HTMLElement* highestAncestorWithConflictingInlineStyle(CSSMutableStyleDeclaration*, Node*);
-    PassRefPtr<CSSMutableStyleDeclaration> extractInlineStyleToPushDown(Node*, const Vector<int>&);
+    PassRefPtr<CSSMutableStyleDeclaration> extractInlineStyleToPushDown(Node*, bool isStyledElement, const Vector<int>&);
     void applyInlineStyleToPushDown(Node*, CSSMutableStyleDeclaration *style);
     void pushDownInlineStyleAroundNode(CSSMutableStyleDeclaration*, Node*);
     void removeInlineStyle(PassRefPtr<CSSMutableStyleDeclaration>, const Position& start, const Position& end);
@@ -92,7 +93,7 @@ private:
     void applyInlineStyle(CSSMutableStyleDeclaration*);
     void applyInlineStyleToRange(CSSMutableStyleDeclaration*, const Position& start, const Position& end);
     void addBlockStyle(const StyleChange&, HTMLElement*);
-    void addInlineStyleIfNeeded(CSSMutableStyleDeclaration*, Node* start, Node* end);
+    void addInlineStyleIfNeeded(CSSMutableStyleDeclaration*, Node* start, Node* end, EAddStyledElement addStyledElement = AddStyledElement);
     void splitTextAtStart(const Position& start, const Position& end);
     void splitTextAtEnd(const Position& start, const Position& end);
     void splitTextElementAtStart(const Position& start, const Position& end);
diff --git a/WebCore/editing/CompositeEditCommand.cpp b/WebCore/editing/CompositeEditCommand.cpp
index f50929a..aa37193 100644
--- a/WebCore/editing/CompositeEditCommand.cpp
+++ b/WebCore/editing/CompositeEditCommand.cpp
@@ -730,30 +730,6 @@ void CompositeEditCommand::pushAnchorElementDown(Node* anchorNode)
         removeNodePreservingChildren(anchorNode);
 }
 
-// We must push partially selected anchors down before creating or removing
-// links from a selection to create fully selected chunks that can be removed.
-// ApplyStyleCommand doesn't do this for us because styles can be nested.
-// Anchors cannot be nested.
-void CompositeEditCommand::pushPartiallySelectedAnchorElementsDown()
-{
-    VisibleSelection originalSelection = endingSelection();
-    VisiblePosition visibleStart(originalSelection.start());
-    VisiblePosition visibleEnd(originalSelection.end());
-    
-    Node* startAnchor = enclosingAnchorElement(originalSelection.start());
-    VisiblePosition startOfStartAnchor(Position(startAnchor, 0));
-    if (startAnchor && startOfStartAnchor != visibleStart)
-        pushAnchorElementDown(startAnchor);
-
-    Node* endAnchor = enclosingAnchorElement(originalSelection.end());
-    VisiblePosition endOfEndAnchor(Position(endAnchor, 0));
-    if (endAnchor && endOfEndAnchor != visibleEnd)
-        pushAnchorElementDown(endAnchor);
-
-    ASSERT(originalSelection.start().node()->inDocument() && originalSelection.end().node()->inDocument());
-    setEndingSelection(originalSelection);
-}
-
 // Clone the paragraph between start and end under blockElement,
 // preserving the hierarchy up to outerNode. 
 
diff --git a/WebCore/editing/CompositeEditCommand.h b/WebCore/editing/CompositeEditCommand.h
index 9d69925..f839a72 100644
--- a/WebCore/editing/CompositeEditCommand.h
+++ b/WebCore/editing/CompositeEditCommand.h
@@ -99,7 +99,6 @@ protected:
     PassRefPtr<Node> moveParagraphContentsToNewBlockIfNecessary(const Position&);
     
     void pushAnchorElementDown(Node*);
-    void pushPartiallySelectedAnchorElementsDown();
     
     void moveParagraph(const VisiblePosition&, const VisiblePosition&, const VisiblePosition&, bool preserveSelection = false, bool preserveStyle = true);
     void moveParagraphs(const VisiblePosition&, const VisiblePosition&, const VisiblePosition&, bool preserveSelection = false, bool preserveStyle = true);
diff --git a/WebCore/editing/CreateLinkCommand.cpp b/WebCore/editing/CreateLinkCommand.cpp
index d7048fe..fe7af4a 100644
--- a/WebCore/editing/CreateLinkCommand.cpp
+++ b/WebCore/editing/CreateLinkCommand.cpp
@@ -46,10 +46,9 @@ void CreateLinkCommand::doApply()
     RefPtr<HTMLAnchorElement> anchorElement = HTMLAnchorElement::create(document());
     anchorElement->setHref(m_url);
     
-    if (endingSelection().isRange()) {
-        pushPartiallySelectedAnchorElementsDown();
+    if (endingSelection().isRange())
         applyStyledElement(anchorElement.get());
-    } else {
+    else {
         insertNodeAt(anchorElement.get(), endingSelection().start());
         RefPtr<Text> textNode = Text::create(document(), m_url);
         appendNode(textNode.get(), anchorElement.get());
diff --git a/WebCore/editing/UnlinkCommand.cpp b/WebCore/editing/UnlinkCommand.cpp
index ca69378..0518838 100644
--- a/WebCore/editing/UnlinkCommand.cpp
+++ b/WebCore/editing/UnlinkCommand.cpp
@@ -41,8 +41,6 @@ void UnlinkCommand::doApply()
     if (!endingSelection().isNonOrphanedRange())
         return;
 
-    pushPartiallySelectedAnchorElementsDown();
-
     removeStyledElement(HTMLAnchorElement::create(document()));
 }
 

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list