From: alexis.menard@openbossa.org Date: Wed, 1 Feb 2012 21:48:23 +0000 (+0000) Subject: CSSStyleDeclaration.getPropertyPriority() fails for CSS shorthand properties with... X-Git-Tag: 070512121124~14016 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a0ad1dc5b54d23bdd64196388efae3681aecae19;p=profile%2Fivi%2Fwebkit-efl.git CSSStyleDeclaration.getPropertyPriority() fails for CSS shorthand properties with 'important' priority 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 --- diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 87f1c63..02d4e6d 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,13 @@ +2012-02-01 Alexis Menard + + 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 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 index 0000000..be366da --- /dev/null +++ b/LayoutTests/fast/css/shorthand-priority-expected.txt @@ -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 index 0000000..b639835 --- /dev/null +++ b/LayoutTests/fast/css/shorthand-priority.html @@ -0,0 +1,38 @@ + + + + + + + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 9012d6c..4abb201 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,31 @@ +2012-02-01 Alexis Menard + + 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 Crash in EventHandler::updateDragAndDrop diff --git a/Source/WebCore/css/CSSMutableStyleDeclaration.cpp b/Source/WebCore/css/CSSMutableStyleDeclaration.cpp index 1511014..c7d5d06 100644 --- a/Source/WebCore/css/CSSMutableStyleDeclaration.cpp +++ b/Source/WebCore/css/CSSMutableStyleDeclaration.cpp @@ -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) diff --git a/Source/WebCore/css/CSSMutableStyleDeclaration.h b/Source/WebCore/css/CSSMutableStyleDeclaration.h index b50d551..73af49f 100644 --- a/Source/WebCore/css/CSSMutableStyleDeclaration.h +++ b/Source/WebCore/css/CSSMutableStyleDeclaration.h @@ -64,7 +64,7 @@ public: PassRefPtr 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; diff --git a/Source/WebCore/editing/EditingStyle.cpp b/Source/WebCore/editing/EditingStyle.cpp index 1bcd69c..7e5441d 100644 --- a/Source/WebCore/editing/EditingStyle.cpp +++ b/Source/WebCore/editing/EditingStyle.cpp @@ -521,9 +521,9 @@ PassRefPtr EditingStyle::extractAndRemoveTextDirection() { RefPtr 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); } } diff --git a/Source/WebCore/editing/RemoveCSSPropertyCommand.cpp b/Source/WebCore/editing/RemoveCSSPropertyCommand.cpp index b1b88a2..5408e39 100644 --- a/Source/WebCore/editing/RemoveCSSPropertyCommand.cpp +++ b/Source/WebCore/editing/RemoveCSSPropertyCommand.cpp @@ -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); } diff --git a/Source/WebKit/qt/Api/qwebelement.cpp b/Source/WebKit/qt/Api/qwebelement.cpp index 6b18994..d527ff9 100644 --- a/Source/WebKit/qt/Api/qwebelement.cpp +++ b/Source/WebKit/qt/Api/qwebelement.cpp @@ -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(rules->item(i - 1)); - if (rule->declaration()->getPropertyPriority(propID)) + if (rule->declaration()->propertyIsImportant(propID)) return rule->declaration()->getPropertyValue(propID); if (style->getPropertyValue(propID).isEmpty()) diff --git a/Source/WebKit/qt/ChangeLog b/Source/WebKit/qt/ChangeLog index ccd9f6a..5543e60 100644 --- a/Source/WebKit/qt/ChangeLog +++ b/Source/WebKit/qt/ChangeLog @@ -1,3 +1,15 @@ +2012-02-01 Alexis Menard + + 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 Try to fix Qt build.