Bug 42764 - Rename CSSStyleSelector to StyleResolver.
: Rename CSSStyleSelector to StyleResolver.
Status: RESOLVED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
: 84734 84765 84814
: 46590
  Show dependency treegraph
 
Reported: 2010-07-21 09:48 PST by
Modified: 2012-04-25 15:40 PST (History)


Attachments
Patch (785.99 KB, patch)
2010-07-21 09:51 PST, Dimitri Glazkov (Google)
no flags Review Patch | Details | Formatted Diff | Diff
Patch (292.29 KB, patch)
2012-04-24 09:27 PST, Alexis Menard (darktears)
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-07-21 09:48:21 PST
Rename CSSStyleSelector to RenderStyleResolver.
------- Comment #1 From 2010-07-21 09:51:54 PST -------
Created an attachment (id=62201) [details]
Patch
------- Comment #2 From 2010-07-21 09:54:32 PST -------
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 From 2010-07-21 09:57:16 PST -------
(From update of attachment 62201 [details])
Please explain the motivation and the benefit of this patch.
------- Comment #4 From 2010-07-21 10:02:30 PST -------
(In reply to comment #3)
> (From update of attachment 62201 [details] [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 From 2010-07-21 10:05:40 PST -------
Another possibility is CSSStyleResolver.
------- Comment #6 From 2010-07-22 09:54:57 PST -------
I’d be fine with CSSStyleResolver, but I’d like to know what Dave Hyatt thinks.
------- Comment #7 From 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 From 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 From 2011-12-04 20:48:30 PST -------
@_@ I can haz review for <style scoped> really, really fast, plz?!?
------- Comment #10 From 2012-04-24 09:27:31 PST -------
Created an attachment (id=138578) [details]
Patch
------- Comment #11 From 2012-04-24 09:32:32 PST -------
(In reply to comment #10)
> Created an attachment (id=138578) [details] [details]
> Patch

As requested I'll make child bugs for each of the patches.
------- Comment #12 From 2012-04-24 10:58:22 PST -------
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 From 2012-04-24 11:10:47 PST -------
(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 From 2012-04-25 15:40:59 PST -------
All patches have landed. Done!