Bug 64782 - Possible unintentional early return in CSSStyleSelector::applyProperty() for CSSPropertyWebkitTextEmphasisStyle.
Summary: Possible unintentional early return in CSSStyleSelector::applyProperty() for ...
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on: 64784 64785 65517
  Show dependency treegraph
Reported: 2011-07-18 22:12 PDT by Luke Macpherson
Modified: 2011-08-02 16:51 PDT (History)
4 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Luke Macpherson 2011-07-18 22:12:47 PDT
Just looking at CSSStyleSelector::applyProperty() for the CSSPropertyWebkitTextEmphasisStyle case, and I see:

HANDLE_INHERIT_AND_INITIAL(textEmphasisFill, TextEmphasisFill)
HANDLE_INHERIT_AND_INITIAL(textEmphasisMark, TextEmphasisMark)
HANDLE_INHERIT_AND_INITIAL(textEmphasisCustomMark, TextEmphasisCustomMark)
if (isInherit || isInitial)

I wanted to check with mitz (svn blame owner) this was the intended behavior - though the more I look at it the more unlikely that seems.
Currently the HANDLE_INHERIT_AND_INITIAL macro will return immediately in the TextEmphasisFill case when (isInherit || isInitial), so the subsequent code is unreachable.

If you can confirm that the intended behavior was to call all three cases, I'll fix it up during an upcoming refactoring.
Comment 1 Luke Macpherson 2011-07-18 22:19:44 PDT
Hmm, I notice the same logical error happens for CSSPropertyWebkitTransformOrigin and CSSPropertyWebkitPerspectiveOrigin have the same issue.

Just goes to show how bad hiding a return statement inside a macro is.
Comment 2 mitz 2011-07-18 22:21:18 PDT
Thanks for noticing this!
Comment 3 Simon Fraser (smfr) 2011-07-18 22:26:58 PDT
I hate those macros!
Comment 4 Luke Macpherson 2011-07-18 22:36:17 PDT
Working on it, hopefully all those macros will be gone soon.