CSSStyleDeclaration.getPropertyPriority() fails for CSS shorthand properties with...
authoralexis.menard@openbossa.org <alexis.menard@openbossa.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Feb 2012 21:48:23 +0000 (21:48 +0000)
committeralexis.menard@openbossa.org <alexis.menard@openbossa.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Feb 2012 21:48:23 +0000 (21:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=49058

Reviewed by Andreas Kling.

Source/WebCore:

CSSMutableStyleDeclaration::getPropertyPriority was not handling shorthands properly. Shorthands are
not part of the property list of the style so we need to query the longhands which are the one added
in the list. Only if the longhands have equal priority the shorthand priority is known. I also renamed
getPropertyPriority (not the CSSOM exposed method) to something more consistent with WebKit naming guidelines.

Test: fast/css/shorthand-priority.html

* css/CSSMutableStyleDeclaration.cpp:
(WebCore::CSSMutableStyleDeclaration::propertyIsImportant):
(WebCore::CSSMutableStyleDeclaration::addParsedProperty):
(WebCore::CSSMutableStyleDeclaration::getPropertyPriority):
* css/CSSMutableStyleDeclaration.h:
(CSSMutableStyleDeclaration):
* editing/EditingStyle.cpp:
(WebCore::EditingStyle::extractAndRemoveTextDirection):
(WebCore::EditingStyle::collapseTextDecorationProperties):
(WebCore::EditingStyle::conflictsWithInlineStyleOfElement):
(WebCore::setTextDecorationProperty):
* editing/RemoveCSSPropertyCommand.cpp:
(WebCore::RemoveCSSPropertyCommand::doApply):

Source/WebKit/qt:

Update the code as getPropertyPriority has been renamed to propertyIsImportant.

* Api/qwebelement.cpp:
(QWebElement::styleProperty):

LayoutTests:

* fast/css/shorthand-priority-expected.txt: Added.
* fast/css/shorthand-priority.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/shorthand-priority-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/shorthand-priority.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSMutableStyleDeclaration.cpp
Source/WebCore/css/CSSMutableStyleDeclaration.h
Source/WebCore/editing/EditingStyle.cpp
Source/WebCore/editing/RemoveCSSPropertyCommand.cpp
Source/WebKit/qt/Api/qwebelement.cpp
Source/WebKit/qt/ChangeLog

index 87f1c63..02d4e6d 100644 (file)
@@ -1,3 +1,13 @@
+2012-02-01  Alexis Menard  <alexis.menard@openbossa.org>
+
+        CSSStyleDeclaration.getPropertyPriority() fails for CSS shorthand properties with 'important' priority
+        https://bugs.webkit.org/show_bug.cgi?id=49058
+
+        Reviewed by Andreas Kling.
+
+        * fast/css/shorthand-priority-expected.txt: Added.
+        * fast/css/shorthand-priority.html: Added.
+
 2012-02-01  Ryosuke Niwa  <rniwa@webkit.org>
 
         Crash in EventHandler::updateDragAndDrop
diff --git a/LayoutTests/fast/css/shorthand-priority-expected.txt b/LayoutTests/fast/css/shorthand-priority-expected.txt
new file mode 100644 (file)
index 0000000..be366da
--- /dev/null
@@ -0,0 +1,15 @@
+Tests that querying the priority for a shorthand works.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS e.style.getPropertyValue('border-bottom-style') is 'solid'
+PASS e.style.getPropertyPriority('border-bottom-style') is 'important'
+PASS e.style.getPropertyValue('border') is '20px solid green'
+PASS e.style.getPropertyPriority('border') is ''
+PASS e.style.getPropertyValue('border') is '20px solid green'
+PASS e.style.getPropertyPriority('border') is 'important'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/css/shorthand-priority.html b/LayoutTests/fast/css/shorthand-priority.html
new file mode 100644 (file)
index 0000000..b639835
--- /dev/null
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+
+description("Tests that querying the priority for a shorthand works.");
+
+var testContainer = document.createElement("div");
+document.body.appendChild(testContainer);
+
+testContainer.innerHTML = '<div id="test">hello</div>';
+
+e = document.getElementById('test');
+
+// Sanity check.
+e.style.setProperty("border-bottom-style", "solid", "!important");
+shouldBe("e.style.getPropertyValue('border-bottom-style')", "'solid'");
+shouldBe("e.style.getPropertyPriority('border-bottom-style')", "'important'");
+
+e.style.borderBottomStyle = "";
+e.style.border = "20px solid green";
+shouldBe("e.style.getPropertyValue('border')", "'20px solid green'");
+shouldBe("e.style.getPropertyPriority('border')", "''");
+
+e.style.border = "";
+e.style.setProperty("border", "20px solid green", "!important");
+shouldBe("e.style.getPropertyValue('border')", "'20px solid green'");
+shouldBe("e.style.getPropertyPriority('border')", "'important'");
+
+document.body.removeChild(testContainer);
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 9012d6c..4abb201 100644 (file)
@@ -1,3 +1,31 @@
+2012-02-01  Alexis Menard  <alexis.menard@openbossa.org>
+
+        CSSStyleDeclaration.getPropertyPriority() fails for CSS shorthand properties with 'important' priority
+        https://bugs.webkit.org/show_bug.cgi?id=49058
+
+        Reviewed by Andreas Kling.
+
+        CSSMutableStyleDeclaration::getPropertyPriority was not handling shorthands properly. Shorthands are
+        not part of the property list of the style so we need to query the longhands which are the one added
+        in the list. Only if the longhands have equal priority the shorthand priority is known. I also renamed
+        getPropertyPriority (not the CSSOM exposed method) to something more consistent with WebKit naming guidelines.
+
+        Test: fast/css/shorthand-priority.html
+
+        * css/CSSMutableStyleDeclaration.cpp:
+        (WebCore::CSSMutableStyleDeclaration::propertyIsImportant):
+        (WebCore::CSSMutableStyleDeclaration::addParsedProperty):
+        (WebCore::CSSMutableStyleDeclaration::getPropertyPriority):
+        * css/CSSMutableStyleDeclaration.h:
+        (CSSMutableStyleDeclaration):
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::extractAndRemoveTextDirection):
+        (WebCore::EditingStyle::collapseTextDecorationProperties):
+        (WebCore::EditingStyle::conflictsWithInlineStyleOfElement):
+        (WebCore::setTextDecorationProperty):
+        * editing/RemoveCSSPropertyCommand.cpp:
+        (WebCore::RemoveCSSPropertyCommand::doApply):
+
 2012-02-01  Ryosuke Niwa  <rniwa@webkit.org>
 
         Crash in EventHandler::updateDragAndDrop
index 1511014..c7d5d06 100644 (file)
@@ -658,10 +658,21 @@ void CSSMutableStyleDeclaration::setNeedsStyleRecalc()
     }
 }
 
-bool CSSMutableStyleDeclaration::getPropertyPriority(int propertyID) const
+bool CSSMutableStyleDeclaration::propertyIsImportant(int propertyID) const
 {
     const CSSProperty* property = findPropertyWithId(propertyID);
-    return property ? property->isImportant() : false;
+    if (property)
+        return property->isImportant();
+
+    CSSPropertyLonghand longhands = longhandForProperty(propertyID);
+    if (!longhands.length())
+        return false;
+
+    for (unsigned i = 0; i < longhands.length(); ++i) {
+        if (!propertyIsImportant(longhands.properties()[i]))
+            return false;
+    }
+    return true;
 }
 
 int CSSMutableStyleDeclaration::getPropertyShorthand(int propertyID) const
@@ -788,7 +799,7 @@ void CSSMutableStyleDeclaration::addParsedProperty(const CSSProperty& property)
 #endif
 
     // Only add properties that have no !important counterpart present
-    if (!getPropertyPriority(property.id()) || property.isImportant()) {
+    if (!propertyIsImportant(property.id()) || property.isImportant()) {
         removeProperty(property.id(), false, false);
         m_properties.append(property);
     }
@@ -1046,7 +1057,7 @@ String CSSMutableStyleDeclaration::getPropertyPriority(const String& propertyNam
     int propertyID = cssPropertyID(propertyName);
     if (!propertyID)
         return String();
-    return getPropertyPriority(propertyID) ? "important" : "";
+    return propertyIsImportant(propertyID) ? "important" : "";
 }
 
 String CSSMutableStyleDeclaration::getPropertyShorthand(const String& propertyName)
index b50d551..73af49f 100644 (file)
@@ -64,7 +64,7 @@ public:
 
     PassRefPtr<CSSValue> getPropertyCSSValue(int propertyID) const;
     String getPropertyValue(int propertyID) const;
-    bool getPropertyPriority(int propertyID) const;
+    bool propertyIsImportant(int propertyID) const;
     int getPropertyShorthand(int propertyID) const;
     bool isPropertyImplicit(int propertyID) const;
 
index 1bcd69c..7e5441d 100644 (file)
@@ -521,9 +521,9 @@ PassRefPtr<EditingStyle> EditingStyle::extractAndRemoveTextDirection()
 {
     RefPtr<EditingStyle> textDirection = EditingStyle::create();
     textDirection->m_mutableStyle = CSSMutableStyleDeclaration::create();
-    textDirection->m_mutableStyle->setProperty(CSSPropertyUnicodeBidi, CSSValueEmbed, m_mutableStyle->getPropertyPriority(CSSPropertyUnicodeBidi));
+    textDirection->m_mutableStyle->setProperty(CSSPropertyUnicodeBidi, CSSValueEmbed, m_mutableStyle->propertyIsImportant(CSSPropertyUnicodeBidi));
     textDirection->m_mutableStyle->setProperty(CSSPropertyDirection, m_mutableStyle->getPropertyValue(CSSPropertyDirection),
-        m_mutableStyle->getPropertyPriority(CSSPropertyDirection));
+        m_mutableStyle->propertyIsImportant(CSSPropertyDirection));
 
     m_mutableStyle->removeProperty(CSSPropertyUnicodeBidi);
     m_mutableStyle->removeProperty(CSSPropertyDirection);
@@ -579,7 +579,7 @@ void EditingStyle::collapseTextDecorationProperties()
         return;
 
     if (textDecorationsInEffect->isValueList())
-        m_mutableStyle->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText(), m_mutableStyle->getPropertyPriority(CSSPropertyTextDecoration));
+        m_mutableStyle->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText(), m_mutableStyle->propertyIsImportant(CSSPropertyTextDecoration));
     else
         m_mutableStyle->removeProperty(CSSPropertyTextDecoration);
     m_mutableStyle->removeProperty(CSSPropertyWebkitTextDecorationsInEffect);
@@ -665,7 +665,7 @@ bool EditingStyle::conflictsWithInlineStyleOfElement(StyledElement* element, Edi
                 return true;
             conflictingProperties->append(CSSPropertyTextDecoration);
             if (extractedStyle)
-                extractedStyle->setProperty(CSSPropertyTextDecoration, inlineStyle->getPropertyValue(CSSPropertyTextDecoration), inlineStyle->getPropertyPriority(CSSPropertyTextDecoration));
+                extractedStyle->setProperty(CSSPropertyTextDecoration, inlineStyle->getPropertyValue(CSSPropertyTextDecoration), inlineStyle->propertyIsImportant(CSSPropertyTextDecoration));
             continue;
         }
 
@@ -677,7 +677,7 @@ bool EditingStyle::conflictsWithInlineStyleOfElement(StyledElement* element, Edi
                 return true;
             conflictingProperties->append(CSSPropertyDirection);
             if (extractedStyle)
-                extractedStyle->setProperty(propertyID, inlineStyle->getPropertyValue(propertyID), inlineStyle->getPropertyPriority(propertyID));
+                extractedStyle->setProperty(propertyID, inlineStyle->getPropertyValue(propertyID), inlineStyle->propertyIsImportant(propertyID));
         }
 
         if (!conflictingProperties)
@@ -686,7 +686,7 @@ bool EditingStyle::conflictsWithInlineStyleOfElement(StyledElement* element, Edi
         conflictingProperties->append(propertyID);
 
         if (extractedStyle)
-            extractedStyle->setProperty(propertyID, inlineStyle->getPropertyValue(propertyID), inlineStyle->getPropertyPriority(propertyID));
+            extractedStyle->setProperty(propertyID, inlineStyle->getPropertyValue(propertyID), inlineStyle->propertyIsImportant(propertyID));
     }
 
     return conflictingProperties && !conflictingProperties->isEmpty();
@@ -1221,10 +1221,10 @@ StyleChange::StyleChange(EditingStyle* style, const Position& position)
 static void setTextDecorationProperty(CSSMutableStyleDeclaration* style, const CSSValueList* newTextDecoration, int propertyID)
 {
     if (newTextDecoration->length())
-        style->setProperty(propertyID, newTextDecoration->cssText(), style->getPropertyPriority(propertyID));
+        style->setProperty(propertyID, newTextDecoration->cssText(), style->propertyIsImportant(propertyID));
     else {
         // text-decoration: none is redundant since it does not remove any text decorations.
-        ASSERT(!style->getPropertyPriority(propertyID));
+        ASSERT(!style->propertyIsImportant(propertyID));
         style->removeProperty(propertyID);
     }
 }
index b1b88a2..5408e39 100644 (file)
@@ -44,7 +44,7 @@ void RemoveCSSPropertyCommand::doApply()
 {
     CSSMutableStyleDeclaration* style = m_element->inlineStyleDecl();
     m_oldValue = style->getPropertyValue(m_property);
-    m_important = style->getPropertyPriority(m_property);
+    m_important = style->propertyIsImportant(m_property);
     style->removeProperty(m_property);
 }
 
index 6b18994..d527ff9 100644 (file)
@@ -850,7 +850,7 @@ QString QWebElement::styleProperty(const QString &name, StyleResolveStrategy str
         return style->getPropertyValue(propID);
 
     if (strategy == CascadedStyle) {
-        if (style->getPropertyPriority(propID))
+        if (style->propertyIsImportant(propID))
             return style->getPropertyValue(propID);
 
         // We are going to resolve the style property by walking through the
@@ -866,7 +866,7 @@ QString QWebElement::styleProperty(const QString &name, StyleResolveStrategy str
             for (int i = rules->length(); i > 0; --i) {
                 CSSStyleRule* rule = static_cast<CSSStyleRule*>(rules->item(i - 1));
 
-                if (rule->declaration()->getPropertyPriority(propID))
+                if (rule->declaration()->propertyIsImportant(propID))
                     return rule->declaration()->getPropertyValue(propID);
 
                 if (style->getPropertyValue(propID).isEmpty())
index ccd9f6a..5543e60 100644 (file)
@@ -1,3 +1,15 @@
+2012-02-01  Alexis Menard  <alexis.menard@openbossa.org>
+
+        CSSStyleDeclaration.getPropertyPriority() fails for CSS shorthand properties with 'important' priority
+        https://bugs.webkit.org/show_bug.cgi?id=49058
+
+        Reviewed by Andreas Kling.
+
+        Update the code as getPropertyPriority has been renamed to propertyIsImportant.
+
+        * Api/qwebelement.cpp:
+        (QWebElement::styleProperty):
+
 2012-01-31  Antti Koivisto  <antti@apple.com>
 
         Try to fix Qt build.