Bug 102446 - Make StyleResolver::applyProperty use isInherit in CSSPropertyWebkitMarquee instead of calculating equivalent in-place.
Summary: Make StyleResolver::applyProperty use isInherit in CSSPropertyWebkitMarquee i...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Luke Macpherson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-15 16:36 PST by Luke Macpherson
Modified: 2012-11-26 13:45 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.60 KB, patch)
2012-11-15 16:38 PST, Luke Macpherson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Macpherson 2012-11-15 16:36:07 PST
Make StyleResolver::applyProperty use isInherit in CSSPropertyWebkitMarquee instead of calculating equivalent in-place.
Comment 1 Luke Macpherson 2012-11-15 16:38:08 PST
Created attachment 174555 [details]
Patch
Comment 2 Alexey Proskuryakov 2012-11-15 23:02:47 PST
Comment on attachment 174555 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174555&action=review

> Source/WebCore/ChangeLog:8
> +        !m_parentNode || !value->isInheritedValue() is equivalent to !isInherit (by De Morgan's law).

Would it make sense to ASSERT it?
Comment 3 Luke Macpherson 2012-11-18 15:09:28 PST
Comment on attachment 174555 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174555&action=review

>> Source/WebCore/ChangeLog:8
>> +        !m_parentNode || !value->isInheritedValue() is equivalent to !isInherit (by De Morgan's law).
> 
> Would it make sense to ASSERT it?

Not really. It's a mathematical certainty. This is likely a case where the original author was simply unaware of isInherit.
Comment 4 Allan Sandfeld Jensen 2012-11-20 13:43:01 PST
Comment on attachment 174555 [details]
Patch

Looks good to me. Not everyday you see mathematical proven patches.
Comment 5 Tony Chang 2012-11-26 13:34:32 PST
Comment on attachment 174555 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174555&action=review

>>> Source/WebCore/ChangeLog:8
>>> +        !m_parentNode || !value->isInheritedValue() is equivalent to !isInherit (by De Morgan's law).
>> 
>> Would it make sense to ASSERT it?
> 
> Not really. It's a mathematical certainty. This is likely a case where the original author was simply unaware of isInherit.

FWIW, refactoring could cause this to get out of sync, so I don't think it would hurt.  That said, we don't do that for other cases in the switch, so it's probably not a big deal.
Comment 6 WebKit Review Bot 2012-11-26 13:44:57 PST
Comment on attachment 174555 [details]
Patch

Clearing flags on attachment: 174555

Committed r135760: <http://trac.webkit.org/changeset/135760>
Comment 7 WebKit Review Bot 2012-11-26 13:45:00 PST
All reviewed patches have been landed.  Closing bug.