|Summary:||Rename CSSStyleSelector to StyleResolver.|
|Product:||WebKit||Reporter:||Dimitri Glazkov (Google) <dglazkov>|
|Component:||CSS||Assignee:||Alexis Menard (darktears) <menard>|
|Severity:||Normal||CC:||arv, eric.carlson, eric, feature-media-reviews, hyatt, kling, koivisto, macpherson, menard, mitz, rakuco, rolandsteiner, simon.fraser, webkit.review.bot|
|Version:||528+ (Nightly build)|
|Bug Depends on:||84734, 84765, 84814|
Description Dimitri Glazkov (Google) 2010-07-21 09:48:21 PDT
Rename CSSStyleSelector to RenderStyleResolver.
Comment 2 WebKit Review Bot 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]  WebCore/css/RenderStyleResolver.h:83: Code inside a namespace should not be indented. [whitespace/indent]  Total errors found: 2 in 94 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 mitz 2010-07-21 09:57:16 PDT
Comment on attachment 62201 [details] Patch Please explain the motivation and the benefit of this patch.
Comment 4 Dimitri Glazkov (Google) 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.
Comment 5 Dimitri Glazkov (Google) 2010-07-21 10:05:40 PDT
Another possibility is CSSStyleResolver.
Comment 6 mitz 2010-07-22 09:54:57 PDT
I’d be fine with CSSStyleResolver, but I’d like to know what Dave Hyatt thinks.
Comment 7 Antti Koivisto 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.
Comment 8 Dimitri Glazkov (Google) 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.
Comment 9 Roland Steiner 2011-12-04 20:48:30 PST
@_@ I can haz review for <style scoped> really, really fast, plz?!?
Comment 10 Alexis Menard (darktears) 2012-04-24 09:27:31 PDT
Created attachment 138578 [details] Patch
Comment 11 Alexis Menard (darktears) 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.
Comment 12 Eric Seidel (no email) 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.
Comment 13 Antti Koivisto 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.
Comment 14 Alexis Menard (darktears) 2012-04-25 15:40:59 PDT
All patches have landed. Done!