WebCore:
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Jun 2009 23:35:24 +0000 (23:35 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Jun 2009 23:35:24 +0000 (23:35 +0000)
2009-06-23  Ojan Vafai  <ojan@chromium.org>

        Reviewed by Dan Bernstein.

        Add logic to CSSStyleDeclaration::diff to deal with font-sizes that are
        keyword values. When diff is called on a CSSStyleDeclaration, we check
        the keywordSize to see if font-size matches a keyword value.

        This ensures that when we diff a CSSMutableStyleDeclaration returned from
        copyInheritableProperties on a CSSComputedStyleDeclaration that we
        correctly identify matching font-sizes.

        https://bugs.webkit.org/show_bug.cgi?id=26279

        Test: editing/inserting/font-size-clears-from-typing-style.html

        * css/CSSComputedStyleDeclaration.cpp:
        (WebCore::CSSComputedStyleDeclaration::cssPropertyMatches):
        * css/CSSComputedStyleDeclaration.h:
        * css/CSSStyleDeclaration.cpp:
        (WebCore::CSSStyleDeclaration::cssPropertyMatches):
        (WebCore::CSSStyleDeclaration::diff):
        * css/CSSStyleDeclaration.h:

LayoutTests:

2009-06-23  Ojan Vafai  <ojan@chromium.org>

        Reviewed by Dan Bernstein.

        This test hits an edge case where typingStyle would never get cleared.
        In addition to making every text insertion go into its own text node,
        this caused large performance problems.

        https://bugs.webkit.org/show_bug.cgi?id=26279

        * editing/execCommand/5770834-1-expected.txt:
            These results match how they were before r43243. It's not clear to me
            why changes to font-size are affecting text-align styling though.
        * editing/inserting/font-size-clears-from-typing-style-expected.txt: Added.
        * editing/inserting/font-size-clears-from-typing-style.html: Added.
        * editing/inserting/resources/TEMPLATE.html: Copied from LayoutTests/editing/execCommand/resources/TEMPLATE.html.
        * editing/inserting/resources/font-size-clears-from-typing-style.js: Added.
        * platform/mac/editing/execCommand/remove-formatting-2-expected.txt:

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/execCommand/5770834-1-expected.txt
LayoutTests/editing/inserting/font-size-clears-from-typing-style-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/font-size-clears-from-typing-style.html [new file with mode: 0644]
LayoutTests/editing/inserting/resources/TEMPLATE.html [new file with mode: 0644]
LayoutTests/editing/inserting/resources/font-size-clears-from-typing-style.js [new file with mode: 0644]
LayoutTests/platform/mac/editing/execCommand/remove-formatting-2-expected.txt
WebCore/ChangeLog
WebCore/css/CSSComputedStyleDeclaration.cpp
WebCore/css/CSSComputedStyleDeclaration.h
WebCore/css/CSSStyleDeclaration.cpp
WebCore/css/CSSStyleDeclaration.h

index f0aa083..d1b6a3c 100644 (file)
@@ -1,3 +1,22 @@
+2009-06-23  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Dan Bernstein.
+
+        This test hits an edge case where typingStyle would never get cleared.
+        In addition to making every text insertion go into its own text node,
+        this caused large performance problems.
+        
+        https://bugs.webkit.org/show_bug.cgi?id=26279
+
+        * editing/execCommand/5770834-1-expected.txt:
+            These results match how they were before r43243. It's not clear to me 
+            why changes to font-size are affecting text-align styling though.
+        * editing/inserting/font-size-clears-from-typing-style-expected.txt: Added.
+        * editing/inserting/font-size-clears-from-typing-style.html: Added.
+        * editing/inserting/resources/TEMPLATE.html: Copied from LayoutTests/editing/execCommand/resources/TEMPLATE.html.
+        * editing/inserting/resources/font-size-clears-from-typing-style.js: Added.
+        * platform/mac/editing/execCommand/remove-formatting-2-expected.txt:
+
 2009-06-23  Kevin McCullough  <kmccullough@apple.com>
 
         Reviewed by Darin Adler.
index 320d877..5fe97fd 100644 (file)
@@ -2,6 +2,6 @@ This tests for a crash when removing format from two paragraphs that are inside
 
 
 <div style="text-align: right;">
-<div>foo<br><div style="text-align: auto;">bar</div></div>
+<div style="text-align: auto;">foo<br><div style="text-align: auto;">bar</div></div>
 </div>
 
diff --git a/LayoutTests/editing/inserting/font-size-clears-from-typing-style-expected.txt b/LayoutTests/editing/inserting/font-size-clears-from-typing-style-expected.txt
new file mode 100644 (file)
index 0000000..70d719d
--- /dev/null
@@ -0,0 +1,11 @@
+Tests that font-size in typingStyle is correctly cleared. See https://bugs.webkit.org/show_bug.cgi?id=26279.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS 1 is 1
+PASS 3 is 3
+PASS successfullyParsed is true
+
+TEST COMPLETE
+BA
diff --git a/LayoutTests/editing/inserting/font-size-clears-from-typing-style.html b/LayoutTests/editing/inserting/font-size-clears-from-typing-style.html
new file mode 100644 (file)
index 0000000..738e75c
--- /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="resources/font-size-clears-from-typing-style.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/resources/TEMPLATE.html b/LayoutTests/editing/inserting/resources/TEMPLATE.html
new file mode 100644 (file)
index 0000000..70ce56c
--- /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="YOUR_JS_FILE_HERE"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/editing/inserting/resources/font-size-clears-from-typing-style.js b/LayoutTests/editing/inserting/resources/font-size-clears-from-typing-style.js
new file mode 100644 (file)
index 0000000..49cc711
--- /dev/null
@@ -0,0 +1,19 @@
+description("Tests that font-size in typingStyle is correctly cleared. See https://bugs.webkit.org/show_bug.cgi?id=26279.");
+
+var editable = document.createElement('div');
+editable.contentEditable = true;
+editable.innerHTML = '<br><div id="wrapper">A</div>';
+
+document.body.appendChild(editable);
+editable.focus();
+
+window.getSelection().setBaseAndExtent(editable, 0, editable, 0);
+document.execCommand('ForwardDelete', false, false);
+document.execCommand('InsertText', false, 'B');
+
+// The forward delete leaves the cursor in the "wrapper" div.
+// After typing, there should be exactly one textnode in the wrapper div.
+shouldBe(String(wrapper.childNodes.length), '1');
+shouldBe(String(wrapper.firstChild.nodeType), '3');
+
+var successfullyParsed = true;
index 6c6b525..b4478a6 100644 (file)
@@ -16,9 +16,7 @@ layer at (0,0) size 800x600
           text run at (0,0) width 722: "This tests that RemoveFormat not only removes style from the selected part of the DOM, but that it also applies the"
           text run at (0,18) width 625: "document default style to the selection if that's necessary in order to leave the selected text unstyled."
       RenderBlock {DIV} at (0,52) size 784x18
-        RenderText {#text} at (0,0) size 28x18
-          text run at (0,0) width 28: "This"
-        RenderText {#text} at (28,0) size 277x18
-          text run at (28,0) width 277: " text should look the same as the text above."
+        RenderText {#text} at (0,0) size 305x18
+          text run at (0,0) width 305: "This text should look the same as the text above."
 selection start: position 0 of child 0 {#text} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
 selection end:   position 4 of child 0 {#text} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 97ef31d..41d60e6 100644 (file)
@@ -1,3 +1,27 @@
+2009-06-23  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Dan Bernstein.
+
+        Add logic to CSSStyleDeclaration::diff to deal with font-sizes that are 
+        keyword values. When diff is called on a CSSStyleDeclaration, we check
+        the keywordSize to see if font-size matches a keyword value.
+        
+        This ensures that when we diff a CSSMutableStyleDeclaration returned from
+        copyInheritableProperties on a CSSComputedStyleDeclaration that we 
+        correctly identify matching font-sizes.
+        
+        https://bugs.webkit.org/show_bug.cgi?id=26279
+
+        Test: editing/inserting/font-size-clears-from-typing-style.html
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::CSSComputedStyleDeclaration::cssPropertyMatches):
+        * css/CSSComputedStyleDeclaration.h:
+        * css/CSSStyleDeclaration.cpp:
+        (WebCore::CSSStyleDeclaration::cssPropertyMatches):
+        (WebCore::CSSStyleDeclaration::diff):
+        * css/CSSStyleDeclaration.h:
+
 2009-06-23  Kevin McCullough  <kmccullough@apple.com>
 
         Reviewed by Darin Adler.
index 96aa0d7..b721f70 100644 (file)
@@ -28,6 +28,7 @@
 #include "CSSMutableStyleDeclaration.h"
 #include "CSSPrimitiveValue.h"
 #include "CSSPrimitiveValueMappings.h"
+#include "CSSProperty.h"
 #include "CSSPropertyNames.h"
 #include "CSSReflectValue.h"
 #include "CSSTimingFunctionValue.h"
@@ -1475,6 +1476,22 @@ void CSSComputedStyleDeclaration::removeComputedInheritablePropertiesFrom(CSSMut
     declaration->removePropertiesInSet(inheritableProperties, numInheritableProperties);
 }
 
+bool CSSComputedStyleDeclaration::cssPropertyMatches(const CSSProperty* property) const
+{
+    if (property->id() == CSSPropertyFontSize && property->value()->isPrimitiveValue() && m_node) {
+        m_node->document()->updateLayoutIgnorePendingStylesheets();
+        RenderStyle* style = m_node->computedStyle();
+        if (style && style->fontDescription().keywordSize()) {
+            int sizeValue = cssIdentifierForFontSizeKeyword(style->fontDescription().keywordSize());
+            CSSPrimitiveValue* primitiveValue = static_cast<CSSPrimitiveValue*>(property->value());
+            if (primitiveValue->primitiveType() == CSSPrimitiveValue::CSS_IDENT && primitiveValue->getIdent() == sizeValue)
+                return true;
+        }
+    }
+
+    return CSSStyleDeclaration::cssPropertyMatches(property);
+}
+
 PassRefPtr<CSSMutableStyleDeclaration> CSSComputedStyleDeclaration::copyInheritableProperties() const
 {
     RefPtr<CSSMutableStyleDeclaration> style = copyPropertiesInSet(inheritableProperties, numInheritableProperties);
index 23244e2..6f81b0e 100644 (file)
@@ -59,6 +59,9 @@ public:
 
     static void removeComputedInheritablePropertiesFrom(CSSMutableStyleDeclaration*);
 
+protected:
+    virtual bool cssPropertyMatches(const CSSProperty*) const;
+
 private:
     CSSComputedStyleDeclaration(PassRefPtr<Node>);
 
index a35f817..404a978 100644 (file)
@@ -118,6 +118,12 @@ CSSRule* CSSStyleDeclaration::parentRule() const
     return (parent() && parent()->isRule()) ? static_cast<CSSRule*>(parent()) : 0;
 }
 
+bool CSSStyleDeclaration::cssPropertyMatches(const CSSProperty* property) const
+{
+    RefPtr<CSSValue> value = getPropertyCSSValue(property->id());
+    return value && value->cssText() == property->value()->cssText();
+}
+
 void CSSStyleDeclaration::diff(CSSMutableStyleDeclaration* style) const
 {
     if (!style)
@@ -128,8 +134,7 @@ void CSSStyleDeclaration::diff(CSSMutableStyleDeclaration* style) const
         CSSMutableStyleDeclaration::const_iterator end = style->end();
         for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
             const CSSProperty& property = *it;
-            RefPtr<CSSValue> value = getPropertyCSSValue(property.id());
-            if (value && (value->cssText() == property.value()->cssText()))
+            if (cssPropertyMatches(&property))
                 propertiesToRemove.append(property.id());
         }
     }
index ef4cc21..18493df 100644 (file)
@@ -27,6 +27,7 @@
 namespace WebCore {
 
 class CSSMutableStyleDeclaration;
+class CSSProperty;
 class CSSRule;
 class CSSValue;
 
@@ -72,6 +73,9 @@ public:
 
 protected:
     CSSStyleDeclaration(CSSRule* parentRule = 0);
+
+    virtual bool cssPropertyMatches(const CSSProperty*) const;
+
 };
 
 } // namespace WebCore