Bug 42764 - Rename CSSStyleSelector to StyleResolver.
: Rename CSSStyleSelector to StyleResolver.
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: CSS
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Alexis Menard (darktears)
:
Depends on: 84734 84765 84814
Blocks: 46590
  Show dependency treegraph
 
Reported: 2010-07-21 09:48 PDT by Dimitri Glazkov (Google)
Modified: 2012-04-25 15:40 PDT (History)
14 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2010-07-21 09:48:21 PDT
Rename CSSStyleSelector to RenderStyleResolver.
Comment 1 Dimitri Glazkov (Google) 2010-07-21 09:51:54 PDT
Created attachment 62201 [details]
Patch
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] [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.
Comment 3 mitz@webkit.org 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@webkit.org 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 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!