2010-09-09 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Sep 2010 02:09:41 +0000 (02:09 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Sep 2010 02:09:41 +0000 (02:09 +0000)
        Reviewed by Eric Seidel.

        REGRESSION: applying new font size causes font-size outside selection to change
        https://bugs.webkit.org/show_bug.cgi?id=45026

        The regression was caused by removeImplicitlyStyledElement not converting font size to CSS value properly.
        Namely, it was treating font size as a pixel value for font-size property.  Fixed this by adding fontSizeToCSSValue
        which uses HTMLFontElement::cssValueFromFontSizeNumber and deployed it in removeImplicitlyStyledElement

        Also fixed a bug in StyleChange::extractTextStyles that it ignores font-size property when the values were
        relative terms such as x-small and -webkit-xxx-large. And replaced the logic to convert from pixel font size
        to legacy font size by CSSStyleSelector::legacyFontSize.

        Test: editing/style/push-down-font-styles.html

        * editing/ApplyStyleCommand.cpp:
        (WebCore::StyleChange::extractTextStyles): Fixed a bug. See above.
        (WebCore::stringToCSSValue): Converts string to CSSValue.
        (WebCore::fontSizeToCSSValue): Converts font size number (String) to CSSPrimitiveValue.
        (WebCore::ApplyStyleCommand::removeImplicitlyStyledElement): Calls stringToCSSValue and fontSizeToCSSValue.
        * css/CSSStyleSelector.h: Made fontSizeForKeyword public.
2010-09-09  Kent Tamura  <tkent@chromium.org>

        Unreviewed, test expectation update.

        The baseline image was wrong.  It was exposed by r45431.
        https://bugs.webkit.org/show_bug.cgi?id=45431

        * platform/chromium-linux/svg/dynamic-updates/SVGFEDiffuseLightingElement-lighting-color-css-prop-expected.checksum:
        * platform/chromium-linux/svg/dynamic-updates/SVGFEDiffuseLightingElement-lighting-color-css-prop-expected.png:
        * platform/chromium/test_expectations.txt:

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/deleting/delete-br-012-expected.txt
LayoutTests/editing/deleting/delete-select-all-001-expected.txt
LayoutTests/editing/execCommand/query-font-size-expected.txt
LayoutTests/editing/execCommand/query-font-size.html
LayoutTests/editing/inserting/5994480-2-expected.txt
LayoutTests/editing/style/block-style-004-expected.txt
LayoutTests/editing/style/block-style-005-expected.txt
LayoutTests/editing/style/block-style-006-expected.txt
LayoutTests/editing/style/fontsize-1-expected.txt
LayoutTests/editing/style/push-down-font-styles-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/push-down-font-styles.html [new file with mode: 0644]
LayoutTests/editing/style/script-tests/push-down-font-styles.js [new file with mode: 0644]
WebCore/ChangeLog
WebCore/css/CSSStyleSelector.h
WebCore/editing/ApplyStyleCommand.cpp

index 5baf917..dbb098a 100644 (file)
 
 2010-09-09  Ryosuke Niwa  <rniwa@webkit.org>
 
+        Reviewed by Eric Seidel.
+
+        REGRESSION: applying new font size causes font-size outside selection to change
+        https://bugs.webkit.org/show_bug.cgi?id=45026
+
+        * editing/deleting/delete-br-012-expected.txt: Conflicting font and span elements are merged into one font element.
+        * editing/inserting/5994480-2-expected.txt: Removed redundant size attribute from font element.
+        * editing/deleting/delete-select-all-001-expected.txt: Style spans are replaced by font elements.
+        * editing/style/block-style-004-expected.txt: Ditto.
+        * editing/style/block-style-005-expected.txt: Ditto.
+        * editing/style/block-style-006-expected.txt: Ditto.
+        * editing/style/fontsize-1-expected.txt: Ditto.
+        * editing/execCommand/query-font-size-expected.txt: Ditto; also fixed the test not to add extraneous spans.
+        * editing/style/push-down-font-styles-expected.txt: Added.
+        * editing/style/push-down-font-styles.html: Added.
+        * editing/style/script-tests/push-down-font-styles.js: Added.
+        (testSingleToggle):
+        (selectAll):
+        (selectTest):
+        (selectFirstWord):
+        (selectSecondWord):
+        (selectLastTwoWords):
+
+2010-09-09  Ryosuke Niwa  <rniwa@webkit.org>
+
         Reviewed by James Robinson.
 
         REGRESSION (r67101): query-font-size.html fails on qt and gtk builds
index 6e966d8..115f6db 100644 (file)
@@ -17,12 +17,9 @@ This tests pressing delete key to remove empty quoted line places cursor at the
 |   style="border-style:solid; border-color:blue;"
 |   <font>
 |     class="Apple-style-span"
-|     size="6"
-|     <span>
-|       class="Apple-style-span"
-|       style="font-size: 24px;"
-|       <#selection-caret>
-|       <br>
+|     size="5"
+|     <#selection-caret>
+|     <br>
 |   "
 "
 | "
index c29acce..b2d68f3 100644 (file)
@@ -1,2 +1,2 @@
 
-execDeleteCommand: <span class="Apple-style-span" style="-webkit-border-horizontal-spacing: 2px; -webkit-border-vertical-spacing: 2px; font-size: medium;"><br></span>
+execDeleteCommand: <font class="Apple-style-span" size="3"><span class="Apple-style-span" style="-webkit-border-horizontal-spacing: 2px; -webkit-border-vertical-spacing: 2px;"><br></span></font>
index 9c9aea5..98adf80 100644 (file)
@@ -1,16 +1,16 @@
 test   html    queryCommandValue result
-execCommand('FontSize', -2)    <span class="Apple-style-span" style="font-size: x-small;">test</span>  1
-execCommand('FontSize', -1)    <span class="Apple-style-span" style="font-size: small;">test</span>    2
-execCommand('FontSize', 0)     <span>test</span>       3
-execCommand('FontSize', 1)     <span class="Apple-style-span" style="font-size: x-small;">test</span>  1
-execCommand('FontSize', 2)     <span class="Apple-style-span" style="font-size: small;">test</span>    2
-execCommand('FontSize', 3)     <span>test</span>       3
-execCommand('FontSize', 4)     <span class="Apple-style-span" style="font-size: large;">test</span>    4
-execCommand('FontSize', 5)     <span class="Apple-style-span" style="font-size: x-large;">test</span>  5
-execCommand('FontSize', 6)     <span class="Apple-style-span" style="font-size: xx-large;">test</span> 6
-execCommand('FontSize', 7)     <span class="Apple-style-span" style="font-size: -webkit-xxx-large;">test</span>        7
-execCommand('FontSize', '8px') <span class="Apple-style-span" style="font-size: -webkit-xxx-large;">test</span>        7
-execCommand('FontSize', '2px') <span class="Apple-style-span" style="font-size: small;">test</span>    2
+execCommand('FontSize', -2)    <font class="Apple-style-span" size="1">test</font>     1
+execCommand('FontSize', -1)    <font class="Apple-style-span" size="2">test</font>     2
+execCommand('FontSize', 0)     test    3
+execCommand('FontSize', 1)     <font class="Apple-style-span" size="1">test</font>     1
+execCommand('FontSize', 2)     <font class="Apple-style-span" size="2">test</font>     2
+execCommand('FontSize', 3)     test    3
+execCommand('FontSize', 4)     <font class="Apple-style-span" size="4">test</font>     4
+execCommand('FontSize', 5)     <font class="Apple-style-span" size="5">test</font>     5
+execCommand('FontSize', 6)     <font class="Apple-style-span" size="6">test</font>     6
+execCommand('FontSize', 7)     <font class="Apple-style-span" size="7">test</font>     7
+execCommand('FontSize', '8px') <font class="Apple-style-span" size="7">test</font>     7
+execCommand('FontSize', '2px') <font class="Apple-style-span" size="2">test</font>     2
 manual CSS font-size: 3px      <span style="font-size: 3px">test</span>        1
 manual CSS font-size: 0.2em    <span style="font-size: 0.2em">test</span>      1
 manual CSS font-size: 17px     <span style="font-size: 17px">test</span>       4
@@ -19,13 +19,13 @@ manual CSS font-size: 50px  <span style="font-size: 50px">test</span>       7
 manual CSS font-size: 5em      <span style="font-size: 5em">test</span>        7
 manual CSS font-size: 10px     <span style="font-size: 10px">test</span>       1
 monospace tests to ensure the bug 19161 does not affect queryCommandValue's values
-execCommand('FontSize', -2)    <span class="Apple-style-span" style="font-size: x-small;"><font class="Apple-style-span" face="monospace">test</font></span>   1
-execCommand('FontSize', -1)    <span class="Apple-style-span" style="font-size: small;"><font class="Apple-style-span" face="monospace">test</font></span>     2
-execCommand('FontSize', 0)     <span><font class="Apple-style-span" face="monospace">test</font></span>        3
-execCommand('FontSize', 1)     <span class="Apple-style-span" style="font-size: x-small;"><font class="Apple-style-span" face="monospace">test</font></span>   1
-execCommand('FontSize', 2)     <span class="Apple-style-span" style="font-size: small;"><font class="Apple-style-span" face="monospace">test</font></span>     2
-execCommand('FontSize', 3)     <span><font class="Apple-style-span" face="monospace">test</font></span>        3
-execCommand('FontSize', 4)     <span class="Apple-style-span" style="font-size: large;"><font class="Apple-style-span" face="monospace">test</font></span>     4
-execCommand('FontSize', 5)     <span class="Apple-style-span" style="font-size: x-large;"><font class="Apple-style-span" face="monospace">test</font></span>   5
-execCommand('FontSize', 6)     <span class="Apple-style-span" style="font-size: xx-large;"><font class="Apple-style-span" face="monospace">test</font></span>  6
-execCommand('FontSize', 7)     <span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><font class="Apple-style-span" face="monospace">test</font></span> 7
+execCommand('FontSize', -2)    <font class="Apple-style-span" size="1"><font class="Apple-style-span" face="monospace">test</font></font>      1
+execCommand('FontSize', -1)    <font class="Apple-style-span" size="2"><font class="Apple-style-span" face="monospace">test</font></font>      2
+execCommand('FontSize', 0)     <font class="Apple-style-span" face="monospace">test</font>     3
+execCommand('FontSize', 1)     <font class="Apple-style-span" size="1"><font class="Apple-style-span" face="monospace">test</font></font>      1
+execCommand('FontSize', 2)     <font class="Apple-style-span" size="2"><font class="Apple-style-span" face="monospace">test</font></font>      2
+execCommand('FontSize', 3)     <font class="Apple-style-span" face="monospace">test</font>     3
+execCommand('FontSize', 4)     <font class="Apple-style-span" size="4"><font class="Apple-style-span" face="monospace">test</font></font>      4
+execCommand('FontSize', 5)     <font class="Apple-style-span" size="5"><font class="Apple-style-span" face="monospace">test</font></font>      5
+execCommand('FontSize', 6)     <font class="Apple-style-span" size="6"><font class="Apple-style-span" face="monospace">test</font></font>      6
+execCommand('FontSize', 7)     <font class="Apple-style-span" size="7"><font class="Apple-style-span" face="monospace">test</font></font>      7
index e902299..e7bca73 100644 (file)
@@ -43,17 +43,6 @@ function setFontFamilyOnContent(fontFamily)
     return editableDiv.firstChild;
 }
 
-function wrapInSpanIfNeeded(shouldBeSpan)
-{
-    // Sometimes the result of the ExecCommand will not have a wrapping <span>
-    if (!shouldBeSpan.localName || shouldBeSpan.localName.toLowerCase() != "span") {
-        shouldBeSpan = document.createElement("span");
-        shouldBeSpan.appendChild(editableDiv.firstChild);
-        editableDiv.appendChild(shouldBeSpan);
-    }
-    return shouldBeSpan;
-}
-
 function reportSizeForSpan(span, comment)
 {
     var tableRow = addRow(table);
@@ -67,7 +56,6 @@ function testExecCommandFontSize(size, fontFamily)
 {
     editableDiv.innerHTML = "test";
     var sizedContent = setFontSizeOnContent(size);
-    sizedContent = wrapInSpanIfNeeded(sizedContent);
     if (fontFamily)
         sizedContent = setFontFamilyOnContent(fontFamily);
     var sizeString = (typeof(size) == "string") ? ("'" + size + "'") : size;
index 224592b..9c891c0 100644 (file)
@@ -1 +1 @@
-<font class="Apple-style-span" face="'Lucida Grande'" size="3"><span class="Apple-style-span" style="font-size: 11px;"><br></span></font>
+<font class="Apple-style-span" face="'Lucida Grande'"><span class="Apple-style-span" style="font-size: 11px;"><br></span></font>
index 8beb464..40b2039 100644 (file)
@@ -6,5 +6,5 @@ x
 execInsertParagraphCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div>
 execInsertParagraphCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div><div id="test" class="editing"><br></div>
 execTypeCharacterCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div><div id="test" class="editing">x</div>
-execFontSizeCommand: <div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div>
-execInsertParagraphCommand: <div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div>
+execFontSizeCommand: <div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div>
+execInsertParagraphCommand: <div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div>
index a1701d8..5ba1eff 100644 (file)
@@ -5,5 +5,5 @@ x
 execInsertParagraphCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div>
 execInsertParagraphCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div><div id="test" class="editing"><br></div>
 execTypeCharacterCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div><div id="test" class="editing">x</div>
-execFontSizeCommand: <div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div>
-execDeleteCommand: <div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div>
+execFontSizeCommand: <div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div>
+execDeleteCommand: <div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div>
index dafd5eb..784dd8a 100644 (file)
@@ -6,5 +6,5 @@ x
 execInsertParagraphCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div>
 execInsertParagraphCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div><div id="test" class="editing"><br></div>
 execTypeCharacterCommand: <div id="test" class="editing">x</div><div id="test" class="editing"><br></div><div id="test" class="editing">x</div>
-execFontSizeCommand: <div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div>
-execInsertParagraphCommand: <div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;"><br></span></div><div id="test" class="editing"><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">x</span></div>
+execFontSizeCommand: <div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div>
+execInsertParagraphCommand: <div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7"><br></font></div><div id="test" class="editing"><font class="Apple-style-span" size="7">x</font></div>
index 3efbc45..38db8d7 100644 (file)
@@ -7,49 +7,49 @@ fontsize 2:
 | 
 
 type small:
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: small;"
+|   size="2"
 |   "small<#selection-caret>"
 
 fontsize 4:
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: small;"
+|   size="2"
 |   "small<#selection-caret>"
 
 type medium:
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: small;"
+|   size="2"
 |   "small"
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: large;"
+|   size="4"
 |   "medium<#selection-caret>"
 
 fontsize 6:
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: small;"
+|   size="2"
 |   "small"
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: large;"
+|   size="4"
 |   "medium<#selection-caret>"
 
 type large:
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: small;"
+|   size="2"
 |   "small"
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: large;"
+|   size="4"
 |   "medium"
-| <span>
+| <font>
 |   class="Apple-style-span"
-|   style="font-size: xx-large;"
+|   size="6"
 |   "large<#selection-caret>"
 
 console:
diff --git a/LayoutTests/editing/style/push-down-font-styles-expected.txt b/LayoutTests/editing/style/push-down-font-styles-expected.txt
new file mode 100644 (file)
index 0000000..fa5cec0
--- /dev/null
@@ -0,0 +1,48 @@
+Test to make sure we push down inline styles properly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Font size
+PASS fontsize 2 on all of "hello world" yields "<font class="Apple-style-span" size="2">hello world</font>"
+PASS fontsize 4 on all of "hello world" yields "<font class="Apple-style-span" size="4">hello world</font>"
+PASS fontsize 5 on first word of "hello world" yields "<font class="Apple-style-span" size="5">hello</font> world"
+PASS fontsize 3 on first word of "<font size="7">hello <div>world</div></font>" yields "hello <div style="font-size: -webkit-xxx-large; ">world</div>"
+PASS fontsize 3 on first word of "<font size="7"><div>hello</div><div>world</div></font>" yields "<div>hello</div><div style="font-size: -webkit-xxx-large; ">world</div>"
+PASS fontsize 3 on second word of "<font size="7"><div>hello</div>world</font>" yields "<div style="font-size: -webkit-xxx-large; ">hello</div>world"
+PASS fontsize 7 on all of "<font size="7"><div>hello</div>world</font>" yields "<font size="7"><div>hello</div>world</font>"
+PASS fontsize 7 on all of "<font size="3"><div>hello</div>world</font>" yields "<div><font class="Apple-style-span" size="7">hello</font></div><font class="Apple-style-span" size="7">world</font>"
+PASS fontsize 6 on all of "<font size="7"><div>hello</div>world</font>" yields "<div><font class="Apple-style-span" size="6">hello</font></div><font class="Apple-style-span" size="6">world</font>"
+PASS fontsize 5 on all of "<font size="7"><div>hello</div>world</font>" yields "<div><font class="Apple-style-span" size="5">hello</font></div><font class="Apple-style-span" size="5">world</font>"
+PASS fontsize 3 on all of "<font size="7"><div>hello</div>world</font>" yields "<div>hello</div>world"
+PASS fontsize 3 on all of "<font size="3"><div>hello</div>world</font>" yields "<font size="3"><div>hello</div>world</font>"
+PASS fontsize 1 on all of "<font size="3"><div>hello</div>world</font>" yields "<div><font class="Apple-style-span" size="1">hello</font></div><font class="Apple-style-span" size="1">world</font>"
+
+Font size (with CSS)
+PASS fontsize 7 on all of "<font size="7"><div>hello</div>world</font>" yields "<font size="7"><div>hello</div>world</font>"
+PASS fontsize 7 on all of "<font size="3"><div>hello</div>world</font>" yields "<div><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">hello</span></div><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">world</span>"
+PASS fontsize 6 on all of "<font size="7"><div>hello</div>world</font>" yields "<div><span class="Apple-style-span" style="font-size: xx-large;">hello</span></div><span class="Apple-style-span" style="font-size: xx-large;">world</span>"
+PASS fontsize 5 on all of "<font size="7"><div>hello</div>world</font>" yields "<div><span class="Apple-style-span" style="font-size: x-large;">hello</span></div><span class="Apple-style-span" style="font-size: x-large;">world</span>"
+PASS fontsize 3 on all of "<font size="7"><div>hello</div>world</font>" yields "<div>hello</div>world"
+PASS fontsize 3 on all of "<font size="3"><div>hello</div>world</font>" yields "<font size="3"><div>hello</div>world</font>"
+PASS fontsize 1 on all of "<font size="3"><div>hello</div>world</font>" yields "<div><span class="Apple-style-span" style="font-size: x-small;">hello</span></div><span class="Apple-style-span" style="font-size: x-small;">world</span>"
+
+Font family
+PASS fontname Arial on all of "hello world" yields "<font class="Apple-style-span" face="Arial">hello world</font>"
+PASS fontname Arial on first word of "<font face="sans-serif">hello world</font>" yields "<font class="Apple-style-span" face="Arial">hello</font><font face="sans-serif"> world</font>"
+PASS fontname Arial on first word of "<font face="sans-serif">hello<div>world</div></font>" yields "<font class="Apple-style-span" face="Arial">hello</font><div style="font-family: sans-serif; ">world</div>"
+PASS fontname Arial on second word of "<font face="sans-serif">hello<div>world</div></font>" yields "<font class="Apple-style-span" face="sans-serif">hello</font><div><font class="Apple-style-span" face="Arial">world</font></div>"
+PASS fontname Sans-Serif on all of "<font face="sans-serif"><div>hello</div><div>world</div></font>" yields "<font face="sans-serif"><div>hello</div><div>world</div></font>"
+PASS fontname Arial on all of "<font face="sans-serif"><div>hello</div><div>world</div></font>" yields "<div><font class="Apple-style-span" face="Arial">hello</font></div><div><font class="Apple-style-span" face="Arial">world</font></div>"
+
+Font family (with CSS)
+PASS fontname Arial on all of "hello world" yields "<span class="Apple-style-span" style="font-family: Arial;">hello world</span>"
+PASS fontname Arial on first word of "<font face="sans-serif">hello world</font>" yields "<span class="Apple-style-span" style="font-family: Arial;">hello</span><font face="sans-serif"> world</font>"
+PASS fontname Arial on first word of "<font face="sans-serif">hello<div>world</div></font>" yields "<span class="Apple-style-span" style="font-family: Arial;">hello</span><div style="font-family: sans-serif; ">world</div>"
+PASS fontname Arial on second word of "<font face="sans-serif">hello<div>world</div></font>" yields "<span class="Apple-style-span" style="font-family: sans-serif;">hello</span><div><span class="Apple-style-span" style="font-family: Arial;">world</span></div>"
+PASS fontname Sans-Serif on all of "<font face="sans-serif"><div>hello</div><div>world</div></font>" yields "<font face="sans-serif"><div>hello</div><div>world</div></font>"
+PASS fontname Arial on all of "<font face="sans-serif"><div>hello</div><div>world</div></font>" yields "<div><span class="Apple-style-span" style="font-family: Arial;">hello</span></div><div><span class="Apple-style-span" style="font-family: Arial;">world</span></div>"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/style/push-down-font-styles.html b/LayoutTests/editing/style/push-down-font-styles.html
new file mode 100644 (file)
index 0000000..ac218df
--- /dev/null
@@ -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/push-down-font-styles.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/editing/style/script-tests/push-down-font-styles.js b/LayoutTests/editing/style/script-tests/push-down-font-styles.js
new file mode 100644 (file)
index 0000000..887df8a
--- /dev/null
@@ -0,0 +1,103 @@
+description('Test to make sure we push down inline styles properly.');
+
+var testContainer = document.createElement("div");
+testContainer.contentEditable = true;
+document.body.appendChild(testContainer);
+
+var styleWithCSS = false;
+
+function testSingleToggle(toggleCommand, value, selector, initialContents, expectedContents)
+{
+    testContainer.innerHTML = initialContents;
+    var selected = selector(testContainer);
+    document.execCommand('styleWithCSS', false, styleWithCSS ? 'true' : 'false');
+    document.execCommand(toggleCommand, false, value);
+    var action = toggleCommand + ' ' + value + ' on ' + selected + ' of "' + initialContents + '" yields "' + testContainer.innerHTML + '"';
+    if (testContainer.innerHTML == expectedContents)
+        testPassed(action);
+    else
+        testFailed(action + ', expected "' + expectedContents + '"');
+}
+
+function selectAll(container) {
+    window.getSelection().selectAllChildren(container);
+    return 'all';
+}
+
+function selectTest(container) {
+    window.getSelection().selectAllChildren(document.getElementById('test'));
+    return 'test';
+}
+
+function selectFirstWord(container) {
+    window.getSelection().setPosition(container, 0);
+    window.getSelection().modify('extend', 'forward', 'word');
+    return 'first word';
+}
+
+function selectSecondWord(container) {
+    window.getSelection().setPosition(container, 0);
+    window.getSelection().modify('move', 'forward', 'word');
+    window.getSelection().modify('extend', 'forward', 'word');
+    return 'second word';
+}
+
+function selectLastTwoWords(container) {
+    window.getSelection().setPosition(container, container.childNodes.length);
+    window.getSelection().modify('extend', 'backward', 'word');
+    window.getSelection().modify('extend', 'backward', 'word');
+    return 'last two words';
+}
+
+
+debug("Font size");
+styleWithCSS = false;
+testSingleToggle("fontsize", 2, selectAll, 'hello world', '<font class="Apple-style-span" size="2">hello world</font>');
+testSingleToggle("fontsize", 4, selectAll, 'hello world', '<font class="Apple-style-span" size="4">hello world</font>');
+testSingleToggle("fontsize", 5, selectFirstWord, 'hello world', '<font class="Apple-style-span" size="5">hello</font> world');
+testSingleToggle("fontsize", 3, selectFirstWord, '<font size="7">hello <div>world</div></font>', 'hello <div style="font-size: -webkit-xxx-large; ">world</div>');
+testSingleToggle("fontsize", 3, selectFirstWord, '<font size="7"><div>hello</div><div>world</div></font>', '<div>hello</div><div style="font-size: -webkit-xxx-large; ">world</div>');
+testSingleToggle("fontsize", 3, selectSecondWord, '<font size="7"><div>hello</div>world</font>', '<div style="font-size: -webkit-xxx-large; ">hello</div>world');
+
+testSingleToggle("fontsize", 7, selectAll, '<font size="7"><div>hello</div>world</font>', '<font size="7"><div>hello</div>world</font>');
+testSingleToggle("fontsize", 7, selectAll, '<font size="3"><div>hello</div>world</font>', '<div><font class="Apple-style-span" size="7">hello</font></div><font class="Apple-style-span" size="7">world</font>');
+testSingleToggle("fontsize", 6, selectAll, '<font size="7"><div>hello</div>world</font>', '<div><font class="Apple-style-span" size="6">hello</font></div><font class="Apple-style-span" size="6">world</font>');
+testSingleToggle("fontsize", 5, selectAll, '<font size="7"><div>hello</div>world</font>', '<div><font class="Apple-style-span" size="5">hello</font></div><font class="Apple-style-span" size="5">world</font>');
+testSingleToggle("fontsize", 3, selectAll, '<font size="7"><div>hello</div>world</font>', '<div>hello</div>world');
+testSingleToggle("fontsize", 3, selectAll, '<font size="3"><div>hello</div>world</font>', '<font size="3"><div>hello</div>world</font>');
+testSingleToggle("fontsize", 1, selectAll, '<font size="3"><div>hello</div>world</font>', '<div><font class="Apple-style-span" size="1">hello</font></div><font class="Apple-style-span" size="1">world</font>');
+
+debug("");
+debug("Font size (with CSS)");
+styleWithCSS = true;
+testSingleToggle("fontsize", 7, selectAll, '<font size="7"><div>hello</div>world</font>', '<font size="7"><div>hello</div>world</font>');
+testSingleToggle("fontsize", 7, selectAll, '<font size="3"><div>hello</div>world</font>', '<div><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">hello</span></div><span class="Apple-style-span" style="font-size: -webkit-xxx-large;">world</span>');
+testSingleToggle("fontsize", 6, selectAll, '<font size="7"><div>hello</div>world</font>', '<div><span class="Apple-style-span" style="font-size: xx-large;">hello</span></div><span class="Apple-style-span" style="font-size: xx-large;">world</span>');
+testSingleToggle("fontsize", 5, selectAll, '<font size="7"><div>hello</div>world</font>', '<div><span class="Apple-style-span" style="font-size: x-large;">hello</span></div><span class="Apple-style-span" style="font-size: x-large;">world</span>');
+testSingleToggle("fontsize", 3, selectAll, '<font size="7"><div>hello</div>world</font>', '<div>hello</div>world');
+testSingleToggle("fontsize", 3, selectAll, '<font size="3"><div>hello</div>world</font>', '<font size="3"><div>hello</div>world</font>');
+testSingleToggle("fontsize", 1, selectAll, '<font size="3"><div>hello</div>world</font>', '<div><span class="Apple-style-span" style="font-size: x-small;">hello</span></div><span class="Apple-style-span" style="font-size: x-small;">world</span>');
+
+debug("");
+debug("Font family");
+styleWithCSS = false;
+testSingleToggle("fontname", "Arial", selectAll, 'hello world', '<font class="Apple-style-span" face="Arial">hello world</font>');
+testSingleToggle("fontname", "Arial", selectFirstWord, '<font face="sans-serif">hello world</font>', '<font class="Apple-style-span" face="Arial">hello</font><font face="sans-serif"> world</font>');
+testSingleToggle("fontname", "Arial", selectFirstWord, '<font face="sans-serif">hello<div>world</div></font>', '<font class="Apple-style-span" face="Arial">hello</font><div style="font-family: sans-serif; ">world</div>');
+testSingleToggle("fontname", "Arial", selectSecondWord, '<font face="sans-serif">hello<div>world</div></font>', '<font class="Apple-style-span" face="sans-serif">hello</font><div><font class="Apple-style-span" face="Arial">world</font></div>');
+testSingleToggle("fontname", "Sans-Serif", selectAll, '<font face="sans-serif"><div>hello</div><div>world</div></font>', '<font face="sans-serif"><div>hello</div><div>world</div></font>');
+testSingleToggle("fontname", "Arial", selectAll, '<font face="sans-serif"><div>hello</div><div>world</div></font>', '<div><font class="Apple-style-span" face="Arial">hello</font></div><div><font class="Apple-style-span" face="Arial">world</font></div>');
+
+debug("");
+debug("Font family (with CSS)");
+styleWithCSS = true;
+testSingleToggle("fontname", "Arial", selectAll, 'hello world', '<span class="Apple-style-span" style="font-family: Arial;">hello world</span>');
+testSingleToggle("fontname", "Arial", selectFirstWord, '<font face="sans-serif">hello world</font>', '<span class="Apple-style-span" style="font-family: Arial;">hello</span><font face="sans-serif"> world</font>');
+testSingleToggle("fontname", "Arial", selectFirstWord, '<font face="sans-serif">hello<div>world</div></font>', '<span class="Apple-style-span" style="font-family: Arial;">hello</span><div style="font-family: sans-serif; ">world</div>');
+testSingleToggle("fontname", "Arial", selectSecondWord, '<font face="sans-serif">hello<div>world</div></font>', '<span class="Apple-style-span" style="font-family: sans-serif;">hello</span><div><span class="Apple-style-span" style="font-family: Arial;">world</span></div>');
+testSingleToggle("fontname", "Sans-Serif", selectAll, '<font face="sans-serif"><div>hello</div><div>world</div></font>', '<font face="sans-serif"><div>hello</div><div>world</div></font>');
+testSingleToggle("fontname", "Arial", selectAll, '<font face="sans-serif"><div>hello</div><div>world</div></font>', '<div><span class="Apple-style-span" style="font-family: Arial;">hello</span></div><div><span class="Apple-style-span" style="font-family: Arial;">world</span></div>');
+
+document.body.removeChild(testContainer);
+
+var successfullyParsed = true;
index a240bef..1b3f458 100644 (file)
@@ -1,3 +1,27 @@
+2010-09-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        REGRESSION: applying new font size causes font-size outside selection to change
+        https://bugs.webkit.org/show_bug.cgi?id=45026
+
+        The regression was caused by removeImplicitlyStyledElement not converting font size to CSS value properly.
+        Namely, it was treating font size as a pixel value for font-size property.  Fixed this by adding fontSizeToCSSValue
+        which uses HTMLFontElement::cssValueFromFontSizeNumber and deployed it in removeImplicitlyStyledElement
+
+        Also fixed a bug in StyleChange::extractTextStyles that it ignores font-size property when the values were
+        relative terms such as x-small and -webkit-xxx-large. And replaced the logic to convert from pixel font size
+        to legacy font size by CSSStyleSelector::legacyFontSize.
+
+        Test: editing/style/push-down-font-styles.html
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::StyleChange::extractTextStyles): Fixed a bug. See above.
+        (WebCore::stringToCSSValue): Converts string to CSSValue.
+        (WebCore::fontSizeToCSSValue): Converts font size number (String) to CSSPrimitiveValue.
+        (WebCore::ApplyStyleCommand::removeImplicitlyStyledElement): Calls stringToCSSValue and fontSizeToCSSValue.
+        * css/CSSStyleSelector.h: Made fontSizeForKeyword public.
+
 2010-09-09  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r67126.
index 4602e6d..4a4565d 100644 (file)
@@ -122,13 +122,14 @@ public:
         PassRefPtr<CSSRuleList> styleRulesForElement(Element*, bool authorOnly);
         PassRefPtr<CSSRuleList> pseudoStyleRulesForElement(Element*, PseudoId, bool authorOnly);
 
+        // Given a CSS keyword in the range (xx-small to -webkit-xxx-large), this function will return
+        // the correct font size scaled relative to the user's default (medium).
+        static float fontSizeForKeyword(Document*, int keyword, bool shouldUseFixedDefaultSize);
+
         // Given a font size in pixel, this function will return legacy font size between 1 and 7.
         static int legacyFontSize(Document*, int pixelFontSize, bool shouldUseFixedDefaultSize);
 
     private:
-        // Given a CSS keyword in the range (xx-small to -webkit-xxx-large), this function will return
-        // the correct font size scaled relative to the user's default (medium).
-        static float fontSizeForKeyword(Document*, int keyword, bool shouldUseFixedDefaultSize);
 
         // When the CSS keyword "larger" is used, this function will attempt to match within the keyword
         // table, and failing that, will simply multiply by 1.2.
index f9cd33a..d104cbe 100644 (file)
 #include "CSSParser.h"
 #include "CSSProperty.h"
 #include "CSSPropertyNames.h"
+#include "CSSStyleSelector.h"
 #include "CSSValueKeywords.h"
 #include "Document.h"
 #include "Editor.h"
 #include "Frame.h"
 #include "HTMLElement.h"
+#include "HTMLFontElement.h"
 #include "HTMLInterchange.h"
 #include "HTMLNames.h"
 #include "NodeList.h"
@@ -110,7 +112,7 @@ public:
 private:
     void init(PassRefPtr<CSSStyleDeclaration>, const Position&);
     void reconcileTextDecorationProperties(CSSMutableStyleDeclaration*);
-    void extractTextStyles(CSSMutableStyleDeclaration*);
+    void extractTextStyles(Document*, CSSMutableStyleDeclaration*, bool shouldUseFixedFontDefautlSize);
 
     String m_cssStyle;
     bool m_applyBold;
@@ -147,7 +149,7 @@ void StyleChange::init(PassRefPtr<CSSStyleDeclaration> style, const Position& po
 
     reconcileTextDecorationProperties(mutableStyle.get());
     if (!document->frame()->editor()->shouldStyleWithCSS())
-        extractTextStyles(mutableStyle.get());
+        extractTextStyles(document, mutableStyle.get(), computedStyle->useFixedFontDefaultSize());
 
     // Changing the whitespace style in a tab span would collapse the tab into a space.
     if (isTabSpanTextNode(position.node()) || isTabSpanNode((position.node())))
@@ -202,7 +204,7 @@ static void setTextDecorationProperty(CSSMutableStyleDeclaration* style, const C
     }
 }
 
-void StyleChange::extractTextStyles(CSSMutableStyleDeclaration* style)
+void StyleChange::extractTextStyles(Document* document, CSSMutableStyleDeclaration* style, bool shouldUseFixedFontDefautlSize)
 {
     ASSERT(style);
 
@@ -259,28 +261,18 @@ void StyleChange::extractTextStyles(CSSMutableStyleDeclaration* style)
             style->removeProperty(CSSPropertyFontSize); // Can't make sense of the number. Put no font size.
         else {
             CSSPrimitiveValue* value = static_cast<CSSPrimitiveValue*>(fontSize.get());
-
-            // Only accept absolute scale
             if (value->primitiveType() >= CSSPrimitiveValue::CSS_PX && value->primitiveType() <= CSSPrimitiveValue::CSS_PC) {
-                float number = value->getFloatValue(CSSPrimitiveValue::CSS_PX);
-                if (number <= 9)
-                    m_applyFontSize = "1";
-                else if (number <= 10)
-                    m_applyFontSize = "2";
-                else if (number <= 13)
-                    m_applyFontSize = "3";
-                else if (number <= 16)
-                    m_applyFontSize = "4";
-                else if (number <= 18)
-                    m_applyFontSize = "5";
-                else if (number <= 24)
-                    m_applyFontSize = "6";
-                else
-                    m_applyFontSize = "7";
+                int pixelFontSize = value->getFloatValue(CSSPrimitiveValue::CSS_PX);
+                int legacyFontSize = CSSStyleSelector::legacyFontSize(document, pixelFontSize, shouldUseFixedFontDefautlSize);
+                // Use legacy font size only if pixel value matches exactly to that of legacy font size.
+                if (CSSStyleSelector::fontSizeForKeyword(document, legacyFontSize - 1 + CSSValueXSmall, shouldUseFixedFontDefautlSize) == pixelFontSize) {
+                    m_applyFontSize = String::number(legacyFontSize);
+                    style->removeProperty(CSSPropertyFontSize);
+                }
+            } else if (CSSValueXSmall <= value->getIdent() && value->getIdent() <= CSSValueWebkitXxxLarge) {
+                m_applyFontSize = String::number(value->getIdent() - CSSValueXSmall + 1);
+                style->removeProperty(CSSPropertyFontSize);
             }
-            // Huge quirk in Microsoft Entourage is that they understand CSS font-size, but also write 
-            // out legacy 1-7 values in font tags (I guess for mailers that are not CSS-savvy at all, 
-            // like Eudora). Yes, they write out *both*. We need to write out both as well.
         }
     }
 }
@@ -1158,33 +1150,52 @@ struct HTMLEquivalent {
     int primitiveId;
     const QualifiedName* element;
     const QualifiedName* attribute;
+    PassRefPtr<CSSValue> (*attributeToCSSValue)(int propertyID, const String&);
     EPushDownType pushDownType;
 };
 
+static PassRefPtr<CSSValue> stringToCSSValue(int propertyID, const String& value)
+{
+    RefPtr<CSSMutableStyleDeclaration> dummyStyle;
+    dummyStyle = CSSMutableStyleDeclaration::create();
+    dummyStyle->setProperty(propertyID, value);
+    return dummyStyle->getPropertyCSSValue(propertyID);
+}
+
+static PassRefPtr<CSSValue> fontSizeToCSSValue(int propertyID, const String& value)
+{
+    UNUSED_PARAM(propertyID);
+    ASSERT(propertyID == CSSPropertyFontSize);
+    int size;
+    if (!HTMLFontElement::cssValueFromFontSizeNumber(value, size))
+        return 0;
+    return CSSPrimitiveValue::createIdentifier(size);
+}
+
 static const HTMLEquivalent HTMLEquivalents[] = {
-    { CSSPropertyFontWeight, false, CSSValueBold, &bTag, 0, ShouldBePushedDown },
-    { CSSPropertyFontWeight, false, CSSValueBold, &strongTag, 0, ShouldBePushedDown },
-    { CSSPropertyVerticalAlign, false, CSSValueSub, &subTag, 0, ShouldBePushedDown },
-    { CSSPropertyVerticalAlign, false, CSSValueSuper, &supTag, 0, ShouldBePushedDown },
-    { CSSPropertyFontStyle, false, CSSValueItalic, &iTag, 0, ShouldBePushedDown },
-    { CSSPropertyFontStyle, false, CSSValueItalic, &emTag, 0, ShouldBePushedDown },
+    { CSSPropertyFontWeight, false, CSSValueBold, &bTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyFontWeight, false, CSSValueBold, &strongTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyVerticalAlign, false, CSSValueSub, &subTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyVerticalAlign, false, CSSValueSuper, &supTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyFontStyle, false, CSSValueItalic, &iTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyFontStyle, false, CSSValueItalic, &emTag, 0, 0, ShouldBePushedDown },
 
     // text-decorations should be CSSValueList
-    { CSSPropertyTextDecoration, true, CSSValueUnderline, &uTag, 0, ShouldBePushedDown },
-    { CSSPropertyTextDecoration, true, CSSValueLineThrough, &sTag, 0, ShouldBePushedDown },
-    { CSSPropertyTextDecoration, true, CSSValueLineThrough, &strikeTag, 0, ShouldBePushedDown },
-    { CSSPropertyWebkitTextDecorationsInEffect, true, CSSValueUnderline, &uTag, 0, ShouldBePushedDown },
-    { CSSPropertyWebkitTextDecorationsInEffect, true, CSSValueLineThrough, &sTag, 0, ShouldBePushedDown },
-    { CSSPropertyWebkitTextDecorationsInEffect, true, CSSValueLineThrough, &strikeTag, 0, ShouldBePushedDown },
+    { CSSPropertyTextDecoration, true, CSSValueUnderline, &uTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyTextDecoration, true, CSSValueLineThrough, &sTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyTextDecoration, true, CSSValueLineThrough, &strikeTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyWebkitTextDecorationsInEffect, true, CSSValueUnderline, &uTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyWebkitTextDecorationsInEffect, true, CSSValueLineThrough, &sTag, 0, 0, ShouldBePushedDown },
+    { CSSPropertyWebkitTextDecorationsInEffect, true, CSSValueLineThrough, &strikeTag, 0, 0, ShouldBePushedDown },
 
     // FIXME: font attributes should only be removed if values were different
-    { CSSPropertyColor, false, CSSValueInvalid, &fontTag, &colorAttr, ShouldBePushedDown },
-    { CSSPropertyFontFamily, false, CSSValueInvalid, &fontTag, &faceAttr, ShouldBePushedDown },
-    { CSSPropertyFontSize, false, CSSValueInvalid, &fontTag, &sizeAttr, ShouldBePushedDown },
+    { CSSPropertyColor, false, CSSValueInvalid, &fontTag, &colorAttr, stringToCSSValue, ShouldBePushedDown },
+    { CSSPropertyFontFamily, false, CSSValueInvalid, &fontTag, &faceAttr, stringToCSSValue, ShouldBePushedDown },
+    { CSSPropertyFontSize, false, CSSValueInvalid, &fontTag, &sizeAttr, fontSizeToCSSValue, ShouldBePushedDown },
 
     // unicode-bidi and direction are pushed down separately so don't push down with other styles.
-    { CSSPropertyUnicodeBidi, false, CSSValueInvalid, 0, &dirAttr, ShouldNotBePushedDown },
-    { CSSPropertyDirection, false, CSSValueInvalid, 0, &dirAttr, ShouldNotBePushedDown },
+    { CSSPropertyDirection, false, CSSValueInvalid, 0, &dirAttr, stringToCSSValue, ShouldNotBePushedDown },
+    { CSSPropertyUnicodeBidi, false, CSSValueInvalid, 0, &dirAttr, stringToCSSValue, ShouldNotBePushedDown },
 };
 
 bool ApplyStyleCommand::removeImplicitlyStyledElement(CSSMutableStyleDeclaration* style, HTMLElement* element, InlineStyleRemovalMode mode, CSSMutableStyleDeclaration* extractedStyle)
@@ -1203,19 +1214,19 @@ bool ApplyStyleCommand::removeImplicitlyStyledElement(CSSMutableStyleDeclaration
         RefPtr<CSSValue> styleValue = style->getPropertyCSSValue(equivalent.propertyID);
         if (!styleValue)
             continue;
-        RefPtr<CSSPrimitiveValue> mapValue = CSSPrimitiveValue::createIdentifier(equivalent.primitiveId);
+        RefPtr<CSSValue> mapValue;
+        if (equivalent.attribute)
+            mapValue = equivalent.attributeToCSSValue(equivalent.propertyID, element->getAttribute(*equivalent.attribute));
+        else
+            mapValue = CSSPrimitiveValue::createIdentifier(equivalent.primitiveId).get();
 
         if (equivalent.isValueList && styleValue->isValueList() && static_cast<CSSValueList*>(styleValue.get())->hasValue(mapValue.get()))
             continue; // If CSS value assumes CSSValueList, then only skip if the value was present in style to apply.
-        else if (styleValue->cssText() == mapValue->cssText())
+        else if (mapValue && styleValue->cssText() == mapValue->cssText())
             continue; // If CSS value is primitive, then skip if they are equal.
 
-        if (extractedStyle) {
-            if (equivalent.primitiveId == CSSValueInvalid)
-                extractedStyle->setProperty(equivalent.propertyID, element->getAttribute(*equivalent.attribute));
-            else
-                extractedStyle->setProperty(equivalent.propertyID, mapValue->cssText());
-        }
+        if (extractedStyle)
+            extractedStyle->setProperty(equivalent.propertyID, mapValue->cssText());
 
         if (mode == RemoveNone)
             return true;