WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED CONFIGURATION CHANGED
113292
Move box-shadow/webkit-box-shadow/text-shadow code to StyleBuilder
https://bugs.webkit.org/show_bug.cgi?id=113292
Summary
Move box-shadow/webkit-box-shadow/text-shadow code to StyleBuilder
Emil A Eklund
Reported
2013-03-26 03:46:29 PDT
Move the CSSPropertyBoxShadow, CSSPropertyTextShadow and CSSPropertyWebkitBoxShadow applying logic out of StyleResolver::applyProperty.
Attachments
Patch
(11.51 KB, patch)
2013-03-26 03:51 PDT
,
Emil A Eklund
koivisto
: review-
koivisto
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Emil A Eklund
Comment 1
2013-03-26 03:51:54 PDT
Created
attachment 195053
[details]
Patch
Antti Koivisto
Comment 2
2013-03-27 05:46:08 PDT
The refactored code is less understandable than the current one. It would be nice to refactor StyleBuilder to be more sane before moving anything more there.
Emil A Eklund
Comment 3
2013-03-27 06:23:58 PDT
(In reply to
comment #2
)
> The refactored code is less understandable than the current one. It would be nice to refactor StyleBuilder to be more sane before moving anything more there.
Is there anything in particular you find harder to understand or just the general concept of using templates this way? I'll gladly change it to make it easier to understand but knowing what you find hard to follow would makes that a lot easier. Thanks.
Antti Koivisto
Comment 4
2013-03-29 07:16:42 PDT
This factors a relatively straightforward switch branch into something that involves templates and function pointers without any obvious gain. How is this a progression?
Antti Koivisto
Comment 5
2013-03-29 07:21:24 PDT
An obvious first step for cleaning this up would be to move StyleResolver::applyProperty and pals to StyleBuilder. Then we could start figuring out how to have a one sane path of doing this.
Allan Sandfeld Jensen
Comment 6
2013-04-04 05:50:13 PDT
Comment on
attachment 195053
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=195053&action=review
> Source/WebCore/css/StyleBuilder.cpp:1274 > + static void applyInheritValue(CSSPropertyID, StyleResolver* styleResolver) > + { > + const ShadowData* shadowData = (styleResolver->parentStyle()->*getterFunction)(); > + (styleResolver->style()->*setterFunction)(shadowData ? adoptPtr(new ShadowData(*shadowData)) : nullptr, 0 /* add */); > + }
I agree with antti. High order functions are cool and all, but it doesn't really make it any cleaner here. These methods all already have the CSSPropertyID, so you could use that to select the methods like the original code.
Brent Fulgham
Comment 7
2022-07-13 14:29:34 PDT
This code has been significantly refactored since this patch was proposed. There doesn't seem to be any action we can take here.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug