Changeset 70283 in webkit


Ignore:
Timestamp:
Oct 21, 2010 6:30:04 PM (14 years ago)
Author:
rniwa@webkit.org
Message:

2010-10-21 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

removeFormat needs to be reimplemented
​https://bugs.webkit.org/show_bug.cgi?id=43017

Reimplemented execCommand('RemoveFormat', false, null). New implementation removes
the same elements removed by Internet Explorer. Because WebKit supports StyleWithCSS
we also reset any editing styles to match that of the root editable element
while Internet Explorer does not remove any CSS styles.

New implementation uses ApplyStyleCommand to remove appropriate elements and reset the style.
Added new constructor and member variable to ApplyStyleCommand to support mass-removal of elements
since it's inefficient to call ApplyStyleCommand on each element we're removing.

To avoid an infinite loop in pushDownInlineStyleAroundNode when mass-removing, WebKit no longer
push down element one level at a time. Instead, we keep a stack of styled elements to be applied,
and apply wrap siblings of targetNode's ancestors by all of them at once.

Tests: editing/execCommand/remove-format-elements.html

editing/execCommand/remove-format-multiple-elements.html

  • editing/ApplyStyleCommand.cpp: (WebCore::ApplyStyleCommand::ApplyStyleCommand): Added; this version takes style and a function pointer to a boolean function that determines which element needs to removed, and set m_removeOnly to true. (WebCore::ApplyStyleCommand::doApply): Added support for m_isInlineElementToRemoveFunction. (WebCore::ApplyStyleCommand::applyBlockStyle): Ditto. (WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange): Exits early if m_removeOnly is true. (WebCore::ApplyStyleCommand::isStyledInlineElementToRemove): Added. (WebCore::ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle): Calls isStyledInlineElementToRemove. (WebCore::ApplyStyleCommand::removeInlineStyleFromElement): Ditto. (WebCore::ApplyStyleCommand::removeInlineStyle): Ditto. (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): See above.
  • editing/ApplyStyleCommand.h: (WebCore::ApplyStyleCommand::create): Added.
  • editing/RemoveFormatCommand.cpp: (WebCore::isElementForRemoveFormatCommand): Added. (WebCore::RemoveFormatCommand::doApply): Rewritten.

2010-10-21 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Tony Chang.

removeFormat needs to be reimplemented
​https://bugs.webkit.org/show_bug.cgi?id=43017

  • editing/execCommand/19403-expected.txt: hr element is no longer removed erroneously.
  • editing/execCommand/19403.html: Updated the test description.
  • editing/execCommand/4786404-1-expected.txt: Change in text nodes but identical rendering.
  • editing/execCommand/4786404-2-expected.txt: Ditto.
  • editing/execCommand/4920488-expected.txt: Preserves anchor element on RemoveFormat.
  • editing/execCommand/4920488.html: Updated the test description.
  • editing/execCommand/4920742-1-expected.txt: Preserves div elements.
  • editing/execCommand/5049671.html: Updated the test to dump twice before and after RemoveFormat. This was a test to ensure WebKit removes anchor elements but we no longer removes anchor elements.
  • editing/execCommand/5049671-expected.txt:
  • editing/execCommand/5573879.html: Updated the test description because WebKit no longer removes lists on RemoveFormat.
  • editing/execCommand/5573879-expected.txt:
  • editing/execCommand/5770834-1-expected.txt: Removed redundant text-align property value.
  • editing/execCommand/remove-format-elements-expected.txt: Added.
  • editing/execCommand/remove-format-elements.html: Added.
  • editing/execCommand/remove-format-multiple-elements-expected.txt: Added.
  • editing/execCommand/remove-format-multiple-elements.html: Added.
  • editing/execCommand/remove-formatting-2-expected.txt: Change in text nodes but identical rendering.
  • editing/execCommand/remove-formatting-expected.txt: WebKit no longer removes anchor, table, tbody, tr, and td elements.
  • editing/execCommand/script-tests/remove-format-multiple-elements.js: Added. (testRemoveFormat): (selectAll): (selectSecondWord): (selectFirstTwoWords): (selectLastTwoWords): (selectLastWord):
  • editing/execCommand/script-tests/toggle-link.js: Anchor wraps div instead of div wrapping anchor.
  • editing/execCommand/script-tests/toggle-unlink.js: Ditto.
  • editing/execCommand/toggle-link-expected.txt: Ditto.
  • editing/execCommand/toggle-unlink-expected.txt: Ditto.
  • editing/execCommand/unlink-expected.txt: i wraps div instead of i wrapping anchor.
  • editing/inserting/space-after-removeformat-expected.txt: Editing delegate change.
Location:
trunk
Files:
5 added
25 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r70282 r70283  
     12010-10-21  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        removeFormat needs to be reimplemented
     6        https://bugs.webkit.org/show_bug.cgi?id=43017
     7
     8        * editing/execCommand/19403-expected.txt: hr element is no longer removed erroneously.
     9        * editing/execCommand/19403.html: Updated the test description.
     10        * editing/execCommand/4786404-1-expected.txt: Change in text nodes but identical rendering.
     11        * editing/execCommand/4786404-2-expected.txt: Ditto.
     12        * editing/execCommand/4920488-expected.txt: Preserves anchor element on RemoveFormat.
     13        * editing/execCommand/4920488.html: Updated the test description.
     14        * editing/execCommand/4920742-1-expected.txt: Preserves div elements.
     15        * editing/execCommand/5049671.html: Updated the test to dump twice before and after RemoveFormat.
     16        This was a test to ensure WebKit removes anchor elements but we no longer removes anchor elements.
     17        * editing/execCommand/5049671-expected.txt:
     18        * editing/execCommand/5573879.html: Updated the test description because WebKit no longer removes
     19        lists on RemoveFormat.
     20        * editing/execCommand/5573879-expected.txt:
     21        * editing/execCommand/5770834-1-expected.txt: Removed redundant text-align property value.
     22        * editing/execCommand/remove-format-elements-expected.txt: Added.
     23        * editing/execCommand/remove-format-elements.html: Added.
     24        * editing/execCommand/remove-format-multiple-elements-expected.txt: Added.
     25        * editing/execCommand/remove-format-multiple-elements.html: Added.
     26        * editing/execCommand/remove-formatting-2-expected.txt: Change in text nodes but identical rendering.
     27        * editing/execCommand/remove-formatting-expected.txt: WebKit no longer removes anchor, table,
     28        tbody, tr, and td elements.
     29        * editing/execCommand/script-tests/remove-format-multiple-elements.js: Added.
     30        (testRemoveFormat):
     31        (selectAll):
     32        (selectSecondWord):
     33        (selectFirstTwoWords):
     34        (selectLastTwoWords):
     35        (selectLastWord):
     36        * editing/execCommand/script-tests/toggle-link.js: Anchor wraps div instead of div wrapping anchor.
     37        * editing/execCommand/script-tests/toggle-unlink.js: Ditto.
     38        * editing/execCommand/toggle-link-expected.txt: Ditto.
     39        * editing/execCommand/toggle-unlink-expected.txt: Ditto.
     40        * editing/execCommand/unlink-expected.txt: i wraps div instead of i wrapping anchor.
     41        * editing/inserting/space-after-removeformat-expected.txt: Editing delegate change.
     42
    1432010-10-21  Tony Gentilcore  <tonyg@chromium.org>
    244

  • trunk/LayoutTests/editing/execCommand/19403-expected.txt

    r34399 r70283  
    1 This tests for an ASSERT during a RemoveFormat call when it's called on a selection containing only a horizontal rule. It should not ASSERT. Bug: It should also not remove the horizontal rule.
    2 <br>
     1This tests for an ASSERT during a RemoveFormat call when it's called on a selection containing only a horizontal rule. It should not ASSERT.
     2<hr>

  • trunk/LayoutTests/editing/execCommand/19403.html

    r34399 r70283  
    1 <div id="description">This tests for an ASSERT during a RemoveFormat call when it's called on a selection containing only a horizontal rule.  It should not ASSERT.  Bug: It should also not remove the horizontal rule.</div>
     1<div id="description">This tests for an ASSERT during a RemoveFormat call when it's called on a selection containing only a horizontal rule.  It should not ASSERT.</div>
    22<div id="edit" contentEditable="true"><hr></div>
    33

  • trunk/LayoutTests/editing/execCommand/4786404-1-expected.txt

    r69980 r70283  
    44|   id="div"
    55|   style="font-weight: normal; color: black;"
    6 |   "<#selection-anchor>foo bar baz<#selection-focus>"
     6|   "<#selection-anchor>foo "
     7|   "bar"
     8|   " baz<#selection-focus>"

  • trunk/LayoutTests/editing/execCommand/4786404-2-expected.txt

    r69980 r70283  
    33|   contenteditable="true"
    44|   id="div"
    5 |   "<#selection-anchor>foo bar baz<#selection-focus>"
     5|   "<#selection-anchor>foo"
     6|   " bar baz<#selection-focus>"

  • trunk/LayoutTests/editing/execCommand/4920488-expected.txt

    r69980 r70283  
    1 This tests for a bug in GMail's Editor, they try to extract the contents of a range that has had it's contents removed from the document by an editing command.  You should see 'dogfood' unstyled below.
     1This tests for a bug in GMail's Editor, they try to extract the contents of a range that has had it's contents removed from the document by an editing command.  Since the bug 43017 requires WebKit does not remove anchor elements, div should be empty after the extraction. We currently leave anchor element in the div due to the Bug 47916.
    22
    33After removeFormat:
    4 | "<#selection-anchor>dogfood<#selection-focus>"
     4| "<#selection-anchor>dog"
     5| <a>
     6|   href="http://www.google.com/"
     7|   "food<#selection-focus>"
    58
    69After extractContents():
    7 | "<#selection-anchor>dogfood<#selection-focus>"
     10| ""
     11| <a>
     12|   href="http://www.google.com/"

  • trunk/LayoutTests/editing/execCommand/4920488.html

    r69980 r70283  
    66<script>
    77
    8 Markup.description("This tests for a bug in GMail's Editor, they try to extract the contents of a range that has had it's contents removed from the document by an editing command.  You should see 'dogfood' unstyled below.");
     8Markup.description("This tests for a bug in GMail's Editor, they try to extract the contents of a range that has had it's contents removed from the document by an editing command.  Since the bug 43017 requires WebKit does not remove anchor elements, div should be empty after the extraction. We currently leave anchor element in the div due to the Bug 47916.");
    99
    1010var div = document.getElementById("div");

  • trunk/LayoutTests/editing/execCommand/4920742-1-expected.txt

    r69982 r70283  
    11This tests for a bug where RemoveFormat would reverse the order of paragraphs. Bug: the caret is not on the last line but it should be.
     2| "<#selection-anchor>foo"
    23| <div>
    3 |   "<#selection-anchor>foo"
    44|   <br>
    5 |   <br>
     5| <div>
    66|   <#selection-focus>
    77|   <br>

  • trunk/LayoutTests/editing/execCommand/5049671-expected.txt

    r69982 r70283  
    1 This tests for a bug where Remove Format would fail to remove links that were fully selected.  You should see plain text only in the editable region below.
    2 | "<#selection-anchor>This shouldn't be a link or underlined.<#selection-focus>"
     1This tests was added for a bug where Remove Format would fail to remove links that were fully selected. However, because the bug 43017 requires WebKit does not remove anchor elements, RemoveFormat should NOT remove anchor elements.
     2
     3Before RemoveFormat:
     4| <a>
     5|   href="http://www.google.com/"
     6|   "<#selection-anchor>This shouldn't be a link or underlined.<#selection-focus>"
     7
     8After RemoveFormat:
     9| <a>
     10|   href="http://www.google.com/"
     11|   "<#selection-anchor>This shouldn't be a link or underlined.<#selection-focus>"

  • trunk/LayoutTests/editing/execCommand/5049671.html

    r69982 r70283  
    66<script>
    77
     8Markup.description('This tests was added for a bug where Remove Format would fail to remove links that were fully selected.'
     9 + ' However, because the bug 43017 requires WebKit does not remove anchor elements, RemoveFormat should NOT remove anchor elements.');
     10
    811var div = document.getElementById("div");
    912div.focus();
    1013document.execCommand("SelectAll");
     14
     15Markup.dump(div, 'Before RemoveFormat');
     16
    1117document.execCommand("RemoveFormat");
    1218
    13 Markup.description('This tests for a bug where Remove Format would fail to remove links that were fully selected.  You should see plain text only in the editable region below.');
    14 Markup.dump(div);
     19Markup.dump(div, 'After RemoveFormat');
    1520
    1621</script>

  • trunk/LayoutTests/editing/execCommand/5573879-expected.txt

    r69982 r70283  
    1 This tests to make sure that RemoveFormat destroys lists if they are fully selected.  You should see foo<br>bar below.
    2 | <div>
    3 |   "<#selection-anchor>foo"
    4 |   <br>
    5 |   "bar<#selection-focus>"
     1This tests to make sure that RemoveFormat destroys lists if they are fully selected. However, because the bug 43017 requires WebKit does not destroy lists, "foo" and "bar" should be in a separate list item.
     2| <ul>
     3|   <li>
     4|     "<#selection-anchor>foo"
     5|   <li>
     6|     "bar<#selection-focus>"

  • trunk/LayoutTests/editing/execCommand/5573879.html

    r69982 r70283  
    1212document.execCommand("RemoveFormat");
    1313
    14 Markup.description('This tests to make sure that RemoveFormat destroys lists if they are fully selected.  You should see foo<br>bar below.');
     14Markup.description('This tests to make sure that RemoveFormat destroys lists if they are fully selected.'
     15 + ' However, because the bug 43017 requires WebKit does not destroy lists, "foo" and "bar" should be in a separate list item.');
    1516Markup.dump(div);
    1617

  • trunk/LayoutTests/editing/execCommand/5770834-1-expected.txt

    r66585 r70283  
    33
    44<div style="text-align: right;">
    5 <div style="text-align: -webkit-auto;">foo<br>bar</div>
     5<div>foo<br>bar</div>
    66</div>
    77

  • trunk/LayoutTests/editing/execCommand/remove-formatting-2-expected.txt

    r70031 r70283  
    1010EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
    1111This tests that RemoveFormat not only removes style from the selected part of the DOM, but that it also applies the document default style to the selection if that's necessary in order to leave the selected text unstyled.
    12 | "<#selection-anchor>This<#selection-focus> text should look the same as the text above."
     12| "<#selection-anchor>This<#selection-focus>"
     13| " text should look the same as the text above."

  • trunk/LayoutTests/editing/execCommand/remove-formatting-expected.txt

    r70031 r70283  
    55EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    66EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    7 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 9 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     7EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 3 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    88EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    99EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
    … …  
    1313
    1414markup:
    15 | "<#selection-anchor>foobarbaz"
     15| "
     16"
     17| "<#selection-anchor>foo"
     18| <a>
     19|   href="http://www.google.com/"
     20|   "bar"
     21| "baz"
    1622| <br>
    17 | "foo  bar     baz"
     23| "
     24"
     25| <table>
     26|   border="1"
     27|   <tbody>
     28|     <tr>
     29|       <td>
     30|         "foo"
     31|       <td>
     32|         "bar"
     33|       <td>
     34|         "baz"
     35| "
     36"
     37| "foo"
     38| "bar"
     39| "baz<#selection-focus>"
    1840| <br>
    19 | "foobarbaz<#selection-focus>"
    2041| "
    2142"

  • trunk/LayoutTests/editing/execCommand/script-tests/toggle-link.js

    r66040 r70283  
    4343testSingleToggle("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');
    4444testSingleToggle("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>');
    45 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>');
     45testSingleToggle("createLink", 'hello <a href="http://trac.webkit.org/" style="font-style: italic;"><b>world</b> WebKit</a>', selectFirstTwoWords,
     46    '<a href="http://webkit.org/">hello <b style="font-style: italic; ">world</b></a><a href="http://trac.webkit.org/"><i> WebKit</i></a>');
    4647testSingleToggle("createLink", 'hello <b>world</b> WebKit', selectLastWord, 'hello <b>world</b> <a href="http://webkit.org/">WebKit</a>');
    4748testSingleToggle("createLink", '<u>hello <b>world</b> WebKit</u>', selectLastWord, '<u>hello <b>world</b> <a href="http://webkit.org/">WebKit</a></u>');
    48 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>');
    49 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>');
    50 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>');
     49testSingleToggle("createLink", '<a href="http://trac.webkit.org/"><div>hello</div><div>world</div></a>', selectLastWord,
     50    '<a href="http://trac.webkit.org/"><div>hello</div></a><div><a href="http://webkit.org/">world</a></div>');
     51testSingleToggle("createLink", '<a href="http://trac.webkit.org/" style="font-weight: bold;"><div>hello</div><div>world</div></a>', selectLastWord,
     52    '<a href="http://trac.webkit.org/"><div style="font-weight: bold; ">hello</div></a><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>');
     53testSingleToggle("createLink", '<a href="http://trac.webkit.org/" style="font-weight: bold;"><div style="font-weight: normal;">hello</div><div>world</div></a>', selectLastWord,
     54    '<a href="http://trac.webkit.org/"><div style="font-weight: normal; ">hello</div></a><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>');
    5155
    5256document.body.removeChild(testContainer);

  • trunk/LayoutTests/editing/execCommand/script-tests/toggle-unlink.js

    r69910 r70283  
    6060    selectAll, 'hello<div style="background-color: yellow; ">world</div><span class="Apple-style-span" style="background-color: yellow;">WebKit</span>');
    6161testSingleToggle("unlink", '<a href="http://webkit.org/" style="font-weight: bold;"><div>hello</div><div>world WebKit</div></a>',
    62     selectLastTwoWords, '<div style="font-weight: bold; "><a href="http://webkit.org/">hello</a></div><div style="font-weight: bold; ">world WebKit</div>');
     62    selectLastTwoWords, '<a href="http://webkit.org/"><div style="font-weight: bold; ">hello</div></a><div style="font-weight: bold; ">world WebKit</div>');
    6363testSingleToggle("unlink", '<a href="http://webkit.org/" style="font-weight: bold;"><div style="font-weight: normal;">hello</div><div>world</div></a>',
    64     selectLastWord, '<div style="font-weight: normal; "><a href="http://webkit.org/">hello</a></div><div style="font-weight: bold; ">world</div>');
     64    selectLastWord, '<a href="http://webkit.org/"><div style="font-weight: normal; ">hello</div></a><div style="font-weight: bold; ">world</div>');
    6565
    6666document.body.removeChild(testContainer);

  • trunk/LayoutTests/editing/execCommand/toggle-link-expected.txt

    r66040 r70283  
    1515PASS 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>"
    1616PASS 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>"
    17 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>"
    18 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>"
    19 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>"
     17PASS select last word of "<a href="http://trac.webkit.org/"><div>hello</div><div>world</div></a>" and createLink (http://webkit.org/) yields "<a href="http://trac.webkit.org/"><div>hello</div></a><div><a href="http://webkit.org/">world</a></div>"
     18PASS 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 "<a href="http://trac.webkit.org/"><div style="font-weight: bold; ">hello</div></a><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>"
     19PASS 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 "<a href="http://trac.webkit.org/"><div style="font-weight: normal; ">hello</div></a><div style="font-weight: bold; "><a href="http://webkit.org/">world</a></div>"
    2020PASS successfullyParsed is true
    2121

  • trunk/LayoutTests/editing/execCommand/toggle-unlink-expected.txt

    r69910 r70283  
    1515PASS unlink on all of "<a href="http://webkit.org/" style="background-color: yellow;"><div>hello</div><div>world</div></a>" yields "<div style="background-color: yellow; ">hello</div><div style="background-color: yellow; ">world</div>"
    1616PASS unlink on all of "hello<a href="http://webkit.org/" style="background-color: yellow;"><div>world</div></a>WebKit" yields "hello<div style="background-color: yellow; ">world</div><span class="Apple-style-span" style="background-color: yellow;">WebKit</span>"
    17 PASS unlink on last two words of "<a href="http://webkit.org/" style="font-weight: bold;"><div>hello</div><div>world WebKit</div></a>" yields "<div style="font-weight: bold; "><a href="http://webkit.org/">hello</a></div><div style="font-weight: bold; ">world WebKit</div>"
    18 PASS unlink on last word of "<a href="http://webkit.org/" style="font-weight: bold;"><div style="font-weight: normal;">hello</div><div>world</div></a>" yields "<div style="font-weight: normal; "><a href="http://webkit.org/">hello</a></div><div style="font-weight: bold; ">world</div>"
     17PASS unlink on last two words of "<a href="http://webkit.org/" style="font-weight: bold;"><div>hello</div><div>world WebKit</div></a>" yields "<a href="http://webkit.org/"><div style="font-weight: bold; ">hello</div></a><div style="font-weight: bold; ">world WebKit</div>"
     18PASS unlink on last word of "<a href="http://webkit.org/" style="font-weight: bold;"><div style="font-weight: normal;">hello</div><div>world</div></a>" yields "<a href="http://webkit.org/"><div style="font-weight: normal; ">hello</div></a><div style="font-weight: bold; ">world</div>"
    1919PASS successfullyParsed is true
    2020

  • trunk/LayoutTests/editing/execCommand/unlink-expected.txt

    r68830 r70283  
    6565This paragraph should should end up unlinked.
    6666<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>
    67 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.
     67This paragraph starts with <a href="http://www.google.com"><i>a</i></a><span id="test3start"> link</span> in the middle. Only the 'a' in the previous sentence should be linked after the test.
    6868<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>

  • trunk/LayoutTests/editing/inserting/space-after-removeformat-expected.txt

    r69509 r70283  
    11EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    2 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    3 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 10 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    4 EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of BODY > HTML > #document to 9 of BODY > HTML > #document
    5 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
    62EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    73EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification

  • trunk/WebCore/ChangeLog

    r70282 r70283  
     12010-10-21  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        removeFormat needs to be reimplemented
     6        https://bugs.webkit.org/show_bug.cgi?id=43017
     7
     8        Reimplemented execCommand('RemoveFormat', false, null). New implementation removes
     9        the same elements removed by Internet Explorer. Because WebKit supports StyleWithCSS
     10        we also reset any editing styles to match that of the root editable element
     11        while Internet Explorer does not remove any CSS styles.
     12
     13        New implementation uses ApplyStyleCommand to remove appropriate elements and reset the style.
     14        Added new constructor and member variable to ApplyStyleCommand to support mass-removal of elements
     15        since it's inefficient to call ApplyStyleCommand on each element we're removing.
     16
     17        To avoid an infinite loop in pushDownInlineStyleAroundNode when mass-removing, WebKit no longer
     18        push down element one level at a time. Instead, we keep a stack of styled elements to be applied,
     19        and apply wrap siblings of targetNode's ancestors by all of them at once.
     20
     21        Tests: editing/execCommand/remove-format-elements.html
     22               editing/execCommand/remove-format-multiple-elements.html
     23
     24        * editing/ApplyStyleCommand.cpp:
     25        (WebCore::ApplyStyleCommand::ApplyStyleCommand): Added; this version takes style and a function pointer
     26        to a boolean function that determines which element needs to removed, and set m_removeOnly to true.
     27        (WebCore::ApplyStyleCommand::doApply): Added support for m_isInlineElementToRemoveFunction.
     28        (WebCore::ApplyStyleCommand::applyBlockStyle): Ditto.
     29        (WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange): Exits early if m_removeOnly is true.
     30        (WebCore::ApplyStyleCommand::isStyledInlineElementToRemove): Added.
     31        (WebCore::ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle): Calls isStyledInlineElementToRemove.
     32        (WebCore::ApplyStyleCommand::removeInlineStyleFromElement): Ditto.
     33        (WebCore::ApplyStyleCommand::removeInlineStyle): Ditto.
     34        (WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode): See above.
     35        * editing/ApplyStyleCommand.h:
     36        (WebCore::ApplyStyleCommand::create): Added.
     37        * editing/RemoveFormatCommand.cpp:
     38        (WebCore::isElementForRemoveFormatCommand): Added.
     39        (WebCore::RemoveFormatCommand::doApply): Rewritten.
     40
    1412010-10-21  Tony Gentilcore  <tonyg@chromium.org>
    242

  • trunk/WebCore/editing/ApplyStyleCommand.cpp

    r69910 r70283  
    539539    , m_styledInlineElement(0)
    540540    , m_removeOnly(false)
     541    , m_isInlineElementToRemoveFunction(0)
    541542{
    542543}
    … …  
    552553    , m_styledInlineElement(0)
    553554    , m_removeOnly(false)
     555    , m_isInlineElementToRemoveFunction(0)
    554556{
    555557}
    … …  
    565567    , m_styledInlineElement(element)
    566568    , m_removeOnly(removeOnly)
     569    , m_isInlineElementToRemoveFunction(0)
     570{
     571}
     572
     573ApplyStyleCommand::ApplyStyleCommand(Document* document, CSSStyleDeclaration* style, IsInlineElementToRemoveFunction isInlineElementToRemoveFunction, EditAction editingAction)
     574    : CompositeEditCommand(document)
     575    , m_style(style->makeMutable())
     576    , m_editingAction(editingAction)
     577    , m_propertyLevel(PropertyDefault)
     578    , m_start(endingSelection().start().downstream())
     579    , m_end(endingSelection().end().upstream())
     580    , m_useEndingSelection(true)
     581    , m_styledInlineElement(0)
     582    , m_removeOnly(true)
     583    , m_isInlineElementToRemoveFunction(isInlineElementToRemoveFunction)
    567584{
    568585}
    … …  
    606623            // apply any remaining styles to the inline elements
    607624            // NOTE: hopefully, this string comparison is the same as checking for a non-null diff
    608             if (blockStyle->length() < m_style->length() || m_styledInlineElement) {
     625            if (blockStyle->length() < m_style->length() || m_styledInlineElement || m_isInlineElementToRemoveFunction) {
    609626                RefPtr<CSSMutableStyleDeclaration> inlineStyle = m_style->copy();
    610627                applyRelativeFontStyleChange(inlineStyle.get());
    … …  
    665682        if (styleChange.cssStyle().length() || m_removeOnly) {
    666683            RefPtr<Node> block = enclosingBlock(paragraphStart.deepEquivalent().node());
    667             RefPtr<Node> newBlock = moveParagraphContentsToNewBlockIfNecessary(paragraphStart.deepEquivalent());
    668             if (newBlock)
    669                 block = newBlock;
     684            if (!m_removeOnly) {
     685                RefPtr<Node> newBlock = moveParagraphContentsToNewBlockIfNecessary(paragraphStart.deepEquivalent());
     686                if (newBlock)
     687                    block = newBlock;
     688            }
    670689            ASSERT(block->isHTMLElement());
    671690            if (block->isHTMLElement()) {
    … …  
    11271146void ApplyStyleCommand::applyInlineStyleToNodeRange(CSSMutableStyleDeclaration* style, Node* node, Node* pastEndNode)
    11281147{
     1148    if (m_removeOnly)
     1149        return;
     1150
    11291151    for (Node* next; node && node != pastEndNode; node = next) {
    11301152        next = node->traverseNextNode();
    … …  
    11721194        if (!removeStyleFromRunBeforeApplyingStyle(style, node, runEnd))
    11731195            continue;
    1174         addInlineStyleIfNeeded(style, node, runEnd, m_removeOnly ? DoNotAddStyledElement : AddStyledElement);
    1175     }
     1196        addInlineStyleIfNeeded(style, node, runEnd, AddStyledElement);
     1197    }
     1198}
     1199
     1200bool ApplyStyleCommand::isStyledInlineElementToRemove(Element* element) const
     1201{
     1202    return (m_styledInlineElement && element->hasTagName(m_styledInlineElement->tagQName()))
     1203        || (m_isInlineElementToRemoveFunction && m_isInlineElementToRemoveFunction(element));
    11761204}
    11771205
    … …  
    11841212        if (node->childNodeCount())
    11851213            continue;
     1214        // We don't consider m_isInlineElementToRemoveFunction here because we never apply style when m_isInlineElementToRemoveFunction is specified
    11861215        if (getPropertiesNotIn(style, computedStyle(node).get())->length()
    11871216            || (m_styledInlineElement && !enclosingNodeWithTag(positionBeforeNode(node), m_styledInlineElement->tagQName()))) {
    … …  
    12231252        return false;
    12241253
    1225     if (m_styledInlineElement && element->hasTagName(m_styledInlineElement->tagQName())) {
     1254    if (isStyledInlineElementToRemove(element)) {
    12261255        if (mode == RemoveNone)
    12271256            return true;
    … …  
    15051534    // The outer loop is traversing the tree vertically from highestAncestor to targetNode
    15061535    Node* current = highestAncestor;
     1536    // Along the way, styled elements that contain targetNode are removed and accumulated into elementsToPushDown.
     1537    // Each child of the removed element, exclusing ancestors of targetNode, is then wrapped by clones of elements in elementsToPushDown.
     1538    Vector<RefPtr<Element> > elementsToPushDown;
    15071539    while (current != targetNode) {
    15081540        ASSERT(current);
    … …  
    15121544        Node* lastChild = current->lastChild();
    15131545        RefPtr<StyledElement> styledElement;
    1514         if (current->isStyledElement() && m_styledInlineElement && current->hasTagName(m_styledInlineElement->tagQName()))
     1546        if (current->isStyledElement() && isStyledInlineElementToRemove(static_cast<Element*>(current))) {
    15151547            styledElement = static_cast<StyledElement*>(current);
     1548            elementsToPushDown.append(styledElement);
     1549        }
    15161550        RefPtr<CSSMutableStyleDeclaration> styleToPushDown = CSSMutableStyleDeclaration::create();
    15171551        removeInlineStyleFromElement(style, static_cast<HTMLElement*>(current), RemoveIfNeeded, styleToPushDown.get());
    … …  
    15221556            Node* nextChild = child->nextSibling();
    15231557
    1524             if (child != targetNode && styledElement) {
    1525                 // If child has children, wrap children of child by a clone of the styled element to avoid infinite loop.
    1526                 // Otherwise, wrap the child by the styled element, and we won't fall into an infinite loop.
    1527                 RefPtr<Element> wrapper = styledElement->cloneElementWithoutChildren();
    1528                 ExceptionCode ec = 0;
    1529                 wrapper->removeAttribute(styleAttr, ec);
    1530                 ASSERT(!ec);
    1531                 if (child->firstChild())
    1532                     surroundNodeRangeWithElement(child->firstChild(), child->lastChild(), wrapper);
    1533                 else
     1558            if (!child->contains(targetNode) && elementsToPushDown.size()) {
     1559                for (size_t i = 0; i < elementsToPushDown.size(); i++) {
     1560                    RefPtr<Element> wrapper = elementsToPushDown[i]->cloneElementWithoutChildren();
     1561                    ExceptionCode ec = 0;
     1562                    wrapper->removeAttribute(styleAttr, ec);
     1563                    ASSERT(!ec);
    15341564                    surroundNodeRangeWithElement(child, child, wrapper);
     1565                }
    15351566            }
    15361567
    … …  
    15951626            RefPtr<CSSMutableStyleDeclaration> styleToPushDown;
    15961627            PassRefPtr<Node> childNode = 0;
    1597             if (m_styledInlineElement && elem->hasTagName(m_styledInlineElement->tagQName())) {
     1628            if (isStyledInlineElementToRemove(elem.get())) {
    15981629                styleToPushDown = CSSMutableStyleDeclaration::create();
    15991630                childNode = elem->firstChild();

  • trunk/WebCore/editing/ApplyStyleCommand.h

    r68059 r70283  
    4545    enum InlineStyleRemovalMode { RemoveIfNeeded, RemoveAlways, RemoveNone };
    4646    enum EAddStyledElement { AddStyledElement, DoNotAddStyledElement };
     47    typedef bool (*IsInlineElementToRemoveFunction)(const Element*);
    4748
    4849    static PassRefPtr<ApplyStyleCommand> create(Document* document, CSSStyleDeclaration* style, EditAction action = EditActionChangeAttributes, EPropertyLevel level = PropertyDefault)
    … …  
    5859        return adoptRef(new ApplyStyleCommand(element, removeOnly, action));
    5960    }
     61    static PassRefPtr<ApplyStyleCommand> create(Document* document, CSSStyleDeclaration* style, IsInlineElementToRemoveFunction isInlineElementToRemoveFunction, EditAction action = EditActionChangeAttributes)
     62    {
     63        return adoptRef(new ApplyStyleCommand(document, style, isInlineElementToRemoveFunction, action));
     64    }
    6065
    6166    static RefPtr<CSSMutableStyleDeclaration> removeNonEditingProperties(CSSStyleDeclaration* style);
    … …  
    6671    ApplyStyleCommand(Document*, CSSStyleDeclaration*, const Position& start, const Position& end, EditAction, EPropertyLevel);
    6772    ApplyStyleCommand(PassRefPtr<Element>, bool removeOnly, EditAction);
     73    ApplyStyleCommand(Document*, CSSStyleDeclaration*, bool (*isInlineElementToRemove)(const Element*), EditAction);
    6874
    6975    virtual void doApply();
    … …  
    7379
    7480    // style-removal helpers
     81    bool isStyledInlineElementToRemove(Element*) const;
    7582    bool removeStyleFromRunBeforeApplyingStyle(CSSMutableStyleDeclaration* style, Node*& runStart, Node*& runEnd);
    7683    bool removeInlineStyleFromElement(CSSMutableStyleDeclaration*, HTMLElement*, InlineStyleRemovalMode = RemoveIfNeeded, CSSMutableStyleDeclaration* extractedStyle = 0);
    … …  
    123130    RefPtr<Element> m_styledInlineElement;
    124131    bool m_removeOnly;
     132    IsInlineElementToRemoveFunction m_isInlineElementToRemoveFunction;
    125133};
    126134

  • trunk/WebCore/editing/RemoveFormatCommand.cpp

    r67238 r70283  
    11/*
    22 * Copyright (C) 2007 Apple Computer, Inc.  All rights reserved.
     3 * Copyright (C) 2010 Google Inc. All rights reserved.
    34 *
    45 * Redistribution and use in source and binary forms, with or without
    … …  
    1112 *    documentation and/or other materials provided with the distribution.
    1213 *
    13  * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
    14  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
    15  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
    16  * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
    17  * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
    18  * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
    19  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
    20  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
    21  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
     14 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
     15 * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
     16 * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
     17 * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
     18 * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
     19 * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
     20 * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
     21 * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
     22 * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
    2223 * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
    23  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
     24 * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    2425 */
    2526
    … …  
    2728#include "RemoveFormatCommand.h"
    2829
     30#include "ApplyStyleCommand.h"
    2931#include "CSSComputedStyleDeclaration.h"
    3032#include "CSSMutableStyleDeclaration.h"
     33#include "CSSValueKeywords.h"
    3134#include "Editor.h"
    3235#include "Frame.h"
     36#include "HTMLElement.h"
    3337#include "HTMLNames.h"
    3438#include "VisibleSelection.h"
    … …  
    3640#include "TextIterator.h"
    3741#include "TypingCommand.h"
    38 #include "ApplyStyleCommand.h"
     42#include "htmlediting.h"
    3943
    4044namespace WebCore {
    … …  
    4751}
    4852
     53static bool isElementForRemoveFormatCommand(const Element* element)
     54{
     55    DEFINE_STATIC_LOCAL(HashSet<QualifiedName>, elements, ());
     56    if (elements.isEmpty()) {
     57        elements.add(acronymTag);
     58        elements.add(bTag);
     59        elements.add(bdoTag);
     60        elements.add(bigTag);
     61        elements.add(citeTag);
     62        elements.add(codeTag);
     63        elements.add(dfnTag);
     64        elements.add(emTag);
     65        elements.add(fontTag);
     66        elements.add(iTag);
     67        elements.add(insTag);
     68        elements.add(kbdTag);
     69        elements.add(nobrTag);
     70        elements.add(qTag);
     71        elements.add(sTag);
     72        elements.add(sampTag);
     73        elements.add(smallTag);
     74        elements.add(strikeTag);
     75        elements.add(strongTag);
     76        elements.add(subTag);
     77        elements.add(supTag);
     78        elements.add(ttTag);
     79        elements.add(uTag);
     80        elements.add(varTag);
     81    }
     82    return elements.contains(element->tagQName());
     83}
     84
    4985void RemoveFormatCommand::doApply()
    5086{
    … …  
    5490        return;
    5591
    56     // Make a plain text string from the selection to remove formatting like tables and lists.
    57     String string = plainText(frame->selection()->selection().toNormalizedRange().get());
    58 
    5992    // Get the default style for this editable root, it's the style that we'll give the
    6093    // content that we're operating on.
    … …  
    6295    RefPtr<CSSMutableStyleDeclaration> defaultStyle = ApplyStyleCommand::editingStyleAtPosition(Position(root, 0));
    6396
    64     // Delete the selected content.
    65     // FIXME: We should be able to leave this to insertText, but its delete operation
    66     // doesn't preserve the style we're about to set.
    67     deleteSelection();
    68    
    69     // Delete doesn't remove fully selected lists.
    70     while (breakOutOfEmptyListItem())
    71         ;
    72    
    73     // If the selection was all formatting (like an empty list) the format-less text will
    74     // be empty. Early return since we don't need to do any of the work that follows and
    75     // to avoid the ASSERT that fires if input(...) is called with an empty String.
    76     if (string.isEmpty())
    77         return;
    78    
    79     // Insert the content with the default style.
    80     // See <rdar://problem/5794382> RemoveFormat doesn't always reset text alignment
    81     frame->selection()->setTypingStyle(defaultStyle.release());
    82    
    83     inputText(string, true);
     97    applyCommandToComposite(ApplyStyleCommand::create(document(), defaultStyle.get(), isElementForRemoveFormatCommand, editingAction()));
    8498}
    8599
Note: See TracChangeset for help on using the changeset viewer.