2010-09-09 Ryosuke Niwa <rniwa@webkit.org>

        Reviewed by Darin Adler.

        QueryCommandValue('FontSize') returns pixel values instead of IE font numbers
        https://bugs.webkit.org/show_bug.cgi?id=21033

        Modified selectionStartCSSPropertyValue to return legacy font size instead of pixel size.
        To implement the conversion between pixel font size and legacy font size,
        added legacyFontSize to CSSStyleSelector with a helper static function findNearestLegacyFontSize.

        Fixed a bug in selectionComputedStyle where it obtains the style of the previous editing position
        even when the selection is a range. This change revealed a crash in executeToggleStyleInList,
        which was also fixed.

        Test: editing/execCommand/query-font-size.html

        * css/CSSComputedStyleDeclaration.cpp:
        (WebCore::CSSComputedStyleDeclaration::getFontSizeCSSValuePreferringKeyword): Corrected style.
        (WebCore::CSSComputedStyleDeclaration::useFixedFontDefaultSize): Added.
        * css/CSSComputedStyleDeclaration.h:
        * css/CSSStyleSelector.cpp:
        (WebCore::CSSStyleSelector::fontSizeForKeyword): Renamed fixed/monospace to shouldUseFixedDefaultSize.
        (WebCore::findNearestLegacyFontSize): Added, a helper for legacyFontSize.
        (WebCore::CSSStyleSelector::legacyFontSize): Added.
        * css/CSSStyleSelector.h:
        * editing/Editor.cpp:
        (WebCore::Editor::selectionStartCSSPropertyValue): Added a conversion from pixel to legacy font size.
        * editing/EditorCommand.cpp:
        (WebCore::executeToggleStyleInList): Crash fix.
        * page/Frame.cpp:
        (WebCore::Frame::selectionComputedStyle): See above.
2010-09-09  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Darin Adler.

        queryCommandValue('FontSize') returns pixel values instead of IE font numbers
        https://bugs.webkit.org/show_bug.cgi?id=21033

        Added a test to ensure queryCommandValue('fontSize') returns legacy font size
        for both variable width and fixed width fonts.

        * editing/execCommand/query-font-size-expected.txt: Added.
        * editing/execCommand/query-font-size.html: Added.
        * fast/serializer: Added.


Canonical link: https://commits.webkit.org/57844@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@67102 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,17 @@
+2010-09-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        queryCommandValue('FontSize') returns pixel values instead of IE font numbers
+        https://bugs.webkit.org/show_bug.cgi?id=21033
+
+        Added a test to ensure queryCommandValue('fontSize') returns legacy font size
+        for both variable width and fixed width fonts.
+
+        * editing/execCommand/query-font-size-expected.txt: Added.
+        * editing/execCommand/query-font-size.html: Added.
+        * fast/serializer: Added.
+
 2010-09-09  Tony Chang  <tony@chromium.org>
 
         Reviewed by Dimitri Glazkov.

LayoutTests/editing/execCommand/query-font-size-expected.txt

@@ -0,0 +1,31 @@
+test	html	queryCommandValue result	getComputedStyle result
+execCommand('FontSize', -2)	<span class="Apple-style-span" style="font-size: x-small;">test</span>	1	10px
+execCommand('FontSize', -1)	<span class="Apple-style-span" style="font-size: small;">test</span>	2	13px
+execCommand('FontSize', 0)	<span>test</span>	3	16px
+execCommand('FontSize', 1)	<span class="Apple-style-span" style="font-size: x-small;">test</span>	1	10px
+execCommand('FontSize', 2)	<span class="Apple-style-span" style="font-size: small;">test</span>	2	13px
+execCommand('FontSize', 3)	<span>test</span>	3	16px
+execCommand('FontSize', 4)	<span class="Apple-style-span" style="font-size: large;">test</span>	4	18px
+execCommand('FontSize', 5)	<span class="Apple-style-span" style="font-size: x-large;">test</span>	5	24px
+execCommand('FontSize', 6)	<span class="Apple-style-span" style="font-size: xx-large;">test</span>	6	32px
+execCommand('FontSize', 7)	<span class="Apple-style-span" style="font-size: -webkit-xxx-large;">test</span>	7	48px
+execCommand('FontSize', '8px')	<span class="Apple-style-span" style="font-size: -webkit-xxx-large;">test</span>	7	48px
+execCommand('FontSize', '2px')	<span class="Apple-style-span" style="font-size: small;">test</span>	2	13px
+manual CSS font-size: 3px	<span style="font-size: 3px">test</span>	1	3px
+manual CSS font-size: 0.2em	<span style="font-size: 0.2em">test</span>	1	9px
+manual CSS font-size: 17px	<span style="font-size: 17px">test</span>	4	17px
+manual CSS font-size: 31px	<span style="font-size: 31px">test</span>	6	31px
+manual CSS font-size: 50px	<span style="font-size: 50px">test</span>	7	50px
+manual CSS font-size: 5em	<span style="font-size: 5em">test</span>	7	80px
+manual CSS font-size: 10px	<span style="font-size: 10px">test</span>	1	10px
+monospace tests to show bug 19161
+execCommand('FontSize', -2)	<span class="Apple-style-span" style="font-size: x-small;"><font class="Apple-style-span" face="monospace">test</font></span>	1	10px
+execCommand('FontSize', -1)	<span class="Apple-style-span" style="font-size: small;"><font class="Apple-style-span" face="monospace">test</font></span>	2	13px
+execCommand('FontSize', 0)	<span><font class="Apple-style-span" face="monospace">test</font></span>	3	16px
+execCommand('FontSize', 1)	<span class="Apple-style-span" style="font-size: x-small;"><font class="Apple-style-span" face="monospace">test</font></span>	1	10px
+execCommand('FontSize', 2)	<span class="Apple-style-span" style="font-size: small;"><font class="Apple-style-span" face="monospace">test</font></span>	2	13px
+execCommand('FontSize', 3)	<span><font class="Apple-style-span" face="monospace">test</font></span>	3	16px
+execCommand('FontSize', 4)	<span class="Apple-style-span" style="font-size: large;"><font class="Apple-style-span" face="monospace">test</font></span>	4	18px
+execCommand('FontSize', 5)	<span class="Apple-style-span" style="font-size: x-large;"><font class="Apple-style-span" face="monospace">test</font></span>	5	24px
+execCommand('FontSize', 6)	<span class="Apple-style-span" style="font-size: xx-large;"><font class="Apple-style-span" face="monospace">test</font></span>	6	32px
+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	48px

LayoutTests/editing/execCommand/query-font-size.html

@@ -0,0 +1,126 @@
+<body>
+<script>
+
+function addRow(table)
+{
+    var tableRow = document.createElement("tr");
+    table.appendChild(tableRow);
+    return tableRow;
+}
+
+function addCellWithNodeContents(tableRow, contents)
+{
+    var tableCell = document.createElement("td");
+    tableCell.appendChild(contents);
+    tableRow.appendChild(tableCell);
+    return tableCell;
+}
+
+function addCellWithTextContents(tableRow, contents)
+{
+    return addCellWithNodeContents(tableRow, document.createTextNode(contents));
+}
+
+function addHeaderWithTextContents(tableRow, contents)
+{
+    var tableCell = document.createElement("th");   
+    tableCell.appendChild(document.createTextNode(contents));
+    tableRow.appendChild(tableCell);
+    return tableCell;
+}
+
+function setFontSizeOnContent(size)
+{
+    window.getSelection().selectAllChildren(editableDiv);
+    document.execCommand("FontSize", false, size);
+    return editableDiv.firstChild;
+}
+
+function setFontFamilyOnContent(fontFamily)
+{
+    window.getSelection().selectAllChildren(editableDiv);
+    document.execCommand("FontName", false, 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);
+    addCellWithTextContents(tableRow, comment);
+    addCellWithTextContents(tableRow, span.parentNode.innerHTML); // sill FF has no outerHTML
+
+    window.getSelection().selectAllChildren(span.parentNode);
+    addCellWithTextContents(tableRow, "" + document.queryCommandValue("FontSize"));
+    addCellWithTextContents(tableRow, "" + window.getComputedStyle(span, null).fontSize);
+}
+
+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;
+    reportSizeForSpan(sizedContent, "execCommand('FontSize', " + sizeString + ")");
+}
+
+function testManualFontSize(size)
+{
+    editableDiv.innerHTML = "<span style='font-size: " + size + "'>test</span>";
+    reportSizeForSpan(editableDiv.firstChild, "manual CSS font-size: " + size);
+}
+
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText();
+
+var editableDiv = document.createElement("div");
+editableDiv.contentEditable = true;
+document.body.appendChild(editableDiv);
+
+var table = document.createElement("table");
+table.border = "1px";
+table.width = "100%";
+document.body.appendChild(table);
+
+var tableRow = addRow(table);
+addHeaderWithTextContents(tableRow, "test");
+addHeaderWithTextContents(tableRow, "html");
+addHeaderWithTextContents(tableRow, "queryCommandValue result");
+addHeaderWithTextContents(tableRow, "getComputedStyle result");
+
+for (var size = -2; size < 8; size++) {
+    testExecCommandFontSize(size);
+}
+testExecCommandFontSize("8px");
+testExecCommandFontSize("2px");
+
+testManualFontSize("3px");
+testManualFontSize("0.2em");
+testManualFontSize("17px");
+testManualFontSize("31px");
+testManualFontSize("50px");
+testManualFontSize("5em");
+testManualFontSize("10px");
+
+tableRow = addRow(table);
+addHeaderWithTextContents(tableRow, "monospace tests to show bug 19161");
+
+for (var size = -2; size < 8; size++) {
+    testExecCommandFontSize(size, 'monospace');
+}
+
+document.body.removeChild(editableDiv);
+
+</script>

WebCore/ChangeLog

@@ -1,3 +1,36 @@
+2010-09-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        QueryCommandValue('FontSize') returns pixel values instead of IE font numbers
+        https://bugs.webkit.org/show_bug.cgi?id=21033
+
+        Modified selectionStartCSSPropertyValue to return legacy font size instead of pixel size.
+        To implement the conversion between pixel font size and legacy font size,
+        added legacyFontSize to CSSStyleSelector with a helper static function findNearestLegacyFontSize.
+
+        Fixed a bug in selectionComputedStyle where it obtains the style of the previous editing position
+        even when the selection is a range. This change revealed a crash in executeToggleStyleInList,
+        which was also fixed.
+
+        Test: editing/execCommand/query-font-size.html
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::CSSComputedStyleDeclaration::getFontSizeCSSValuePreferringKeyword): Corrected style.
+        (WebCore::CSSComputedStyleDeclaration::useFixedFontDefaultSize): Added.
+        * css/CSSComputedStyleDeclaration.h:
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::fontSizeForKeyword): Renamed fixed/monospace to shouldUseFixedDefaultSize.
+        (WebCore::findNearestLegacyFontSize): Added, a helper for legacyFontSize.
+        (WebCore::CSSStyleSelector::legacyFontSize): Added.
+        * css/CSSStyleSelector.h:
+        * editing/Editor.cpp:
+        (WebCore::Editor::selectionStartCSSPropertyValue): Added a conversion from pixel to legacy font size.
+        * editing/EditorCommand.cpp:
+        (WebCore::executeToggleStyleInList): Crash fix.
+        * page/Frame.cpp:
+        (WebCore::Frame::selectionComputedStyle): See above.
+
 2010-09-09  Robert Hogan  <robert@webkit.org>
 
         Reviewed by Adam Barth.

WebCore/css/CSSComputedStyleDeclaration.cpp

@@ -571,13 +571,12 @@ static int cssIdentifierForFontSizeKeyword(int keywordSize)
 
 PassRefPtr<CSSValue> CSSComputedStyleDeclaration::getFontSizeCSSValuePreferringKeyword() const
 {
-    Node* node = m_node.get();
-    if (!node)
+    if (!m_node)
         return 0;
 
-    node->document()->updateLayoutIgnorePendingStylesheets();
+    m_node->document()->updateLayoutIgnorePendingStylesheets();
 
-    RefPtr<RenderStyle> style = node->computedStyle(m_pseudoElementSpecifier);
+    RefPtr<RenderStyle> style = m_node->computedStyle(m_pseudoElementSpecifier);
     if (!style)
         return 0;
 
@@ -587,6 +586,18 @@ PassRefPtr<CSSValue> CSSComputedStyleDeclaration::getFontSizeCSSValuePreferringK
     return CSSPrimitiveValue::create(style->fontDescription().computedPixelSize(), CSSPrimitiveValue::CSS_PX);
 }
 
+bool CSSComputedStyleDeclaration::useFixedFontDefaultSize() const
+{
+    if (!m_node)
+        return false;
+
+    RefPtr<RenderStyle> style = m_node->computedStyle(m_pseudoElementSpecifier);
+    if (!style)
+        return false;
+
+    return style->fontDescription().useFixedDefaultSize();
+}
+
 PassRefPtr<CSSValue> CSSComputedStyleDeclaration::valueForShadow(const ShadowData* shadow, int id) const
 {
     if (!shadow)

WebCore/css/CSSComputedStyleDeclaration.h

@@ -58,6 +58,7 @@ class CSSComputedStyleDeclaration : public CSSStyleDeclaration {
 
     PassRefPtr<CSSValue> getPropertyCSSValue(int propertyID, EUpdateLayout) const;
     PassRefPtr<CSSValue> getFontSizeCSSValuePreferringKeyword() const;
+    bool useFixedFontDefaultSize() const;
 #if ENABLE(SVG)
     PassRefPtr<CSSValue> getSVGPropertyCSSValue(int propertyID, EUpdateLayout) const;
 #endif

WebCore/css/CSSStyleSelector.cpp

@@ -6311,14 +6311,14 @@ static const int strictFontSizeTable[fontSizeTableMax - fontSizeTableMin + 1][to
 // factors for each keyword value.
 static const float fontSizeFactors[totalKeywords] = { 0.60f, 0.75f, 0.89f, 1.0f, 1.2f, 1.5f, 2.0f, 3.0f };
 
-float CSSStyleSelector::fontSizeForKeyword(Document* document, int keyword, bool fixed)
+float CSSStyleSelector::fontSizeForKeyword(Document* document, int keyword, bool shouldUseFixedDefaultSize)
 {
     Settings* settings = document->settings();
     if (!settings)
         return 1.0f;
 
     bool quirksMode = document->inQuirksMode();
-    int mediumSize = fixed ? settings->defaultFixedFontSize() : settings->defaultFontSize();
+    int mediumSize = shouldUseFixedDefaultSize ? settings->defaultFixedFontSize() : settings->defaultFontSize();
     if (mediumSize >= fontSizeTableMin && mediumSize <= fontSizeTableMax) {
         // Look up the entry in the table.
         int row = mediumSize - fontSizeTableMin;
@@ -6331,6 +6331,33 @@ float CSSStyleSelector::fontSizeForKeyword(Document* document, int keyword, bool
     return max(fontSizeFactors[keyword - CSSValueXxSmall]*mediumSize, minLogicalSize);
 }
 
+template<typename T>
+static int findNearestLegacyFontSize(int pixelFontSize, const T* table, int multiplier)
+{
+    // Ignore table[0] because xx-small does not correspond to any legacy font size.
+    for (int i = 1; i < totalKeywords - 1; i++) {
+        if (pixelFontSize * 2 < (table[i] + table[i + 1]) * multiplier)
+            return i;
+    }
+    return totalKeywords - 1;
+}
+
+int CSSStyleSelector::legacyFontSize(Document* document, int pixelFontSize, bool shouldUseFixedDefaultSize)
+{
+    Settings* settings = document->settings();
+    if (!settings)
+        return 1;
+
+    bool quirksMode = document->inQuirksMode();
+    int mediumSize = shouldUseFixedDefaultSize ? settings->defaultFixedFontSize() : settings->defaultFontSize();
+    if (mediumSize >= fontSizeTableMin && mediumSize <= fontSizeTableMax) {
+        int row = mediumSize - fontSizeTableMin;
+        return findNearestLegacyFontSize<int>(pixelFontSize, quirksMode ? quirksFontSizeTable[row] : strictFontSizeTable[row], 1);
+    }
+
+    return findNearestLegacyFontSize<float>(pixelFontSize, fontSizeFactors, mediumSize);
+}
+
 float CSSStyleSelector::largerFontSize(float size, bool) const
 {
     // FIXME: Figure out where we fall in the size ranges (xx-small to xxx-large) and scale up to

WebCore/css/CSSStyleSelector.h

@@ -122,10 +122,13 @@ class MediaQueryResult : public Noncopyable {
         PassRefPtr<CSSRuleList> styleRulesForElement(Element*, bool authorOnly);
         PassRefPtr<CSSRuleList> pseudoStyleRulesForElement(Element*, PseudoId, bool authorOnly);
 
+        // 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 monospace);
+        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.

WebCore/editing/Editor.cpp

@@ -33,6 +33,7 @@
 #include "CSSMutableStyleDeclaration.h"
 #include "CSSProperty.h"
 #include "CSSPropertyNames.h"
+#include "CSSStyleSelector.h"
 #include "CSSValueKeywords.h"
 #include "CharacterNames.h"
 #include "ClipboardEvent.h"
@@ -938,7 +939,7 @@ static bool hasTransparentBackgroundColor(CSSStyleDeclaration* style)
 String Editor::selectionStartCSSPropertyValue(int propertyID)
 {
     Node* nodeToRemove;
-    RefPtr<CSSStyleDeclaration> selectionStyle = m_frame->selectionComputedStyle(nodeToRemove);
+    RefPtr<CSSComputedStyleDeclaration> selectionStyle = m_frame->selectionComputedStyle(nodeToRemove);
     if (!selectionStyle)
         return String();
 
@@ -965,6 +966,14 @@ String Editor::selectionStartCSSPropertyValue(int propertyID)
         }
     }
 
+    if (propertyID == CSSPropertyFontSize) {
+        RefPtr<CSSValue> value = selectionStyle->getPropertyCSSValue(CSSPropertyFontSize);
+        ASSERT(value->isPrimitiveValue());
+        int fontPixelSize = static_cast<CSSPrimitiveValue*>(value.get())->getIntValue(CSSPrimitiveValue::CSS_PX);
+        int size = CSSStyleSelector::legacyFontSize(m_frame->document(), fontPixelSize, selectionStyle->useFixedFontDefaultSize());
+        return String::number(size);
+    }
+
     return value;
 }
 

WebCore/editing/EditorCommand.cpp

@@ -133,6 +133,9 @@ static bool executeToggleStyleInList(Frame* frame, EditorCommandSource source, E
     ExceptionCode ec = 0;
     Node* nodeToRemove = 0;
     RefPtr<CSSComputedStyleDeclaration> selectionStyle = frame->selectionComputedStyle(nodeToRemove);
+    if (!selectionStyle)
+        return false;
+
     RefPtr<CSSValue> selectedCSSValue = selectionStyle->getPropertyCSSValue(propertyID);
     String newStyle = "none";
     if (selectedCSSValue->isValueList()) {

WebCore/page/Frame.cpp

@@ -745,6 +745,14 @@ PassRefPtr<CSSComputedStyleDeclaration> Frame::selectionComputedStyle(Node*& nod
     RefPtr<Range> range(selection()->toNormalizedRange());
     Position pos = range->editingStartPosition();
 
+    // If the pos is at the end of a text node, then this node is not fully selected.
+    // Move it to the next deep equivalent position to avoid removing the style from this node.
+    // e.g. if pos was at Position("hello", 5) in <b>hello<div>world</div></b>, we want Position("world", 0) instead.
+    // We only do this for range because caret at Position("hello", 5) in <b>hello</b>world should give you font-weight: bold.
+    Node* posNode = pos.containerNode();
+    if (selection()->isRange() && posNode && posNode->isTextNode() && pos.computeOffsetInContainerNode() == posNode->maxCharacterOffset())
+        pos = nextVisuallyDistinctCandidate(pos);
+
     Element *elem = pos.element();
     if (!elem)
         return 0;