WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
42764
Rename CSSStyleSelector to StyleResolver.
https://bugs.webkit.org/show_bug.cgi?id=42764
Summary
Rename CSSStyleSelector to StyleResolver.
Dimitri Glazkov (Google)
Reported
2010-07-21 09:48:21 PDT
Rename CSSStyleSelector to RenderStyleResolver.
Attachments
Patch
(785.99 KB, patch)
2010-07-21 09:51 PDT
,
Dimitri Glazkov (Google)
no flags
Details
Formatted Diff
Diff
Patch
(292.29 KB, patch)
2012-04-24 09:27 PDT
,
Alexis Menard (darktears)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1
2010-07-21 09:51:54 PDT
Created
attachment 62201
[details]
Patch
WebKit Review Bot
Comment 2
2010-07-21 09:54:32 PDT
Attachment 62201
[details]
did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/css/SVGCSSStyleSelector.cpp:32: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/css/RenderStyleResolver.h:83: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 94 files If any of these errors are false positives, please file a bug against check-webkit-style.
mitz
Comment 3
2010-07-21 09:57:16 PDT
Comment on
attachment 62201
[details]
Patch Please explain the motivation and the benefit of this patch.
Dimitri Glazkov (Google)
Comment 4
2010-07-21 10:02:30 PDT
(In reply to
comment #3
)
> (From update of
attachment 62201
[details]
) > Please explain the motivation and the benefit of this patch.
Ah, good point. I should've included a more detailed ChangeLog entry. Looking at CSSStyleSelector, I understand that its primary use is to determine the RenderStyle for an element (CSSStyleSelector::[pseudo]styleForFoo) -- and a bunch of helpers around that. So RenderStyleResolver seems to fit better. Plus, it seems to me that the name is a bit confusing, since CSSStyleSelector could be interpreted as an encapsulation of CSS Selector, which it's not.
Dimitri Glazkov (Google)
Comment 5
2010-07-21 10:05:40 PDT
Another possibility is CSSStyleResolver.
mitz
Comment 6
2010-07-22 09:54:57 PDT
I’d be fine with CSSStyleResolver, but I’d like to know what Dave Hyatt thinks.
Antti Koivisto
Comment 7
2011-12-04 08:14:10 PST
StyleResolver perhaps. I think CSS prefix should mainly be reserved for cssom types. CSSStyleApplyProperty needs a new name too. StyleBuilder might be good.
Dimitri Glazkov (Google)
Comment 8
2011-12-04 08:16:26 PST
(In reply to
comment #7
)
> StyleResolver perhaps. I think CSS prefix should mainly be reserved for cssom types. > > CSSStyleApplyProperty needs a new name too. StyleBuilder might be good.
I like StyleResolver. And StyleBuilder, for that matter.
Roland Steiner
Comment 9
2011-12-04 20:48:30 PST
@_@ I can haz review for <style scoped> really, really fast, plz?!?
Alexis Menard (darktears)
Comment 10
2012-04-24 09:27:31 PDT
Created
attachment 138578
[details]
Patch
Alexis Menard (darktears)
Comment 11
2012-04-24 09:32:32 PDT
(In reply to
comment #10
)
> Created an attachment (id=138578) [details] > Patch
As requested I'll make child bugs for each of the patches.
Eric Seidel (no email)
Comment 12
2012-04-24 10:58:22 PDT
I think it's more interesting to split out the logic into a new class instead of renaming the existing one. CSSStyleSelector has a whole bunch of per-element and per-selector state. None of which the pure "resolver" would need. One of my goals after I finally land seamless is to rip a bunch of that state out to simplify the object.
Antti Koivisto
Comment 13
2012-04-24 11:10:47 PDT
(In reply to
comment #12
)
> I think it's more interesting to split out the logic into a new class instead of renaming the existing one. CSSStyleSelector has a whole bunch of per-element and per-selector state. None of which the pure "resolver" would need. One of my goals after I finally land seamless is to rip a bunch of that state out to simplify the object.
Sure but that is a completely orthogonal task. We still don't want to have silly names around.
Alexis Menard (darktears)
Comment 14
2012-04-25 15:40:59 PDT
All patches have landed. Done!
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