Summary: | Cleanup CSSStyleApplyProperty.cpp | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||
Component: | CSS | Assignee: | Nikolas Zimmermann <zimmermann> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | eric, koivisto, macpherson | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | 54707 | ||||||
Bug Blocks: | 59774 | ||||||
Attachments: |
|
Description
Nikolas Zimmermann
2011-04-27 11:37:42 PDT
Created attachment 91316 [details]
Patch
r=me Was this just the typedef code taken from my patch https://bug-59414-attachments.webkit.org/attachment.cgi?id=91221 ? View in context: https://bugs.webkit.org/attachment.cgi?id=91316&action=review Overall I like the cleanup. Thanks. > Source/WebCore/css/CSSStyleApplyProperty.cpp:147 > + (selector->style()->*m_setter)(m_initial ? m_initial() : Color()); Looks good. > Source/WebCore/css/CSSStyleApplyProperty.cpp:155 > + if (m_initial && primitiveValue->getIdent() == CSSValueCurrentcolor) Use of m_initial here seems to add an odd side-effect to that parameter. Perhaps this would be better handled by adding another constructor value "inheritColorFromParent = true". (In reply to comment #4) > Was this just the typedef code taken from my patch https://bug-59414-attachments.webkit.org/attachment.cgi?id=91221 ? I started from scratch, I forgot that you partly fixed that problem in your patch! (In reply to comment #5) > View in context: https://bugs.webkit.org/attachment.cgi?id=91316&action=review > > Overall I like the cleanup. Thanks. > > > Source/WebCore/css/CSSStyleApplyProperty.cpp:147 > > + (selector->style()->*m_setter)(m_initial ? m_initial() : Color()); > > Looks good. > > > Source/WebCore/css/CSSStyleApplyProperty.cpp:155 > > + if (m_initial && primitiveValue->getIdent() == CSSValueCurrentcolor) > > Use of m_initial here seems to add an odd side-effect to that parameter. Perhaps this would be better handled by adding another constructor value "inheritColorFromParent = true". That's true, if you want to modify it I'm happy to review! |