| Summary: | Setting inline style to the same value it already has triggers a style recalc | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||
| Component: | CSS | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | kling, koivisto, simon.fraser, webkit-bug-importer, zalan | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
Created attachment 251102 [details]
Patch
Created attachment 251104 [details]
Patch
Comment on attachment 251104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251104&action=review Ok though I would prefer a factoring that does not introduce a new bool& return value. > Source/WebCore/css/CSSParser.cpp:1323 > +bool CSSParser::parseValue(MutableStyleProperties* declaration, CSSPropertyID propertyID, const String& string, bool important, StyleSheetContents* contextStyleSheet, bool& changed) It would be nicer if these function returned an enum instead of two bools: enum class ParseResult { Changed, Unchanged, Error } or similar. > Source/WebCore/css/CSSProperty.h:95 > + if (!m_value && !other.m_value) > + return true; You should do if (m_value == other.m_value) return true; instead. Values may be shared. > Source/WebCore/css/StyleProperties.cpp:683 > // When replacing an existing property value, this moves the property to the end of the list. > // Firefox preserves the position, and MSIE moves the property to the beginning. Is this comment true anymore? The setProperty code looks like it replaces properties in-place.
> > Source/WebCore/css/StyleProperties.cpp:683
> > // When replacing an existing property value, this moves the property to the end of the list.
> > // Firefox preserves the position, and MSIE moves the property to the beginning.
>
> Is this comment true anymore? The setProperty code looks like it replaces
> properties in-place.
Didn't change this because I didn't have time to investigate.
|
PropertySetCSSStyleDeclaration::setPropertyInternal() does: bool changed = m_propertySet->setProperty(propertyID, value, important, contextStyleSheet()); but as far as I can see, setProperty() returns true simply when parsing succeeded, not necessarily when the property changed. This causes lots of extra style recalcs when scrolling Facebook.