Bug 64782
| Summary: | Possible unintentional early return in CSSStyleSelector::applyProperty() for CSSPropertyWebkitTextEmphasisStyle. | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Luke Macpherson <macpherson> |
| Component: | CSS | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | hyatt, macpherson, mitz, simon.fraser |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | 64784, 64785, 65517 | ||
| Bug Blocks: | |||
Luke Macpherson
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)
return;
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.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Luke Macpherson
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.
mitz
Thanks for noticing this!
Simon Fraser (smfr)
I hate those macros!
Luke Macpherson
Working on it, hopefully all those macros will be gone soon.