Bug 72480

Summary: CSSStyleSelector: refactor sorting of matched rules
Product: WebKit Reporter: Roland Steiner <rolandsteiner>
Component: CSSAssignee: Roland Steiner <rolandsteiner>
Status: RESOLVED INVALID    
Severity: Normal CC: dglazkov, dominicc, koivisto, macpherson, morrita, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 49142, 67720    
Attachments:
Description Flags
Patch koivisto: review-, koivisto: commit-queue-

Roland Steiner
Reported 2011-11-16 00:58:24 PST
In CSSStyleSelector, after rules are matched they are sorted according to specificity. This currently happens in 2 different places and thus should be refactored. This will also help in the implementation of <style scoped> (see bug 67720).
Attachments
Patch (6.91 KB, patch)
2011-11-16 02:04 PST, Roland Steiner 		koivisto: review-
koivisto: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Roland Steiner
Comment 1 2011-11-16 01:14:33 PST
:P Turns out, that code has changed since I worked on it (in unsavory ways, IMHO), so marking this as invalid.
Roland Steiner
Comment 2 2011-11-16 02:03:15 PST
Upon further thought, a refactoring may still make sense, esp. in view of <style scoped> later on. It just won't be quite as elegant anymore. :p
Roland Steiner
Comment 3 2011-11-16 02:04:26 PST
Created attachment 115349 [details] Patch
Antti Koivisto
Comment 4 2011-11-16 05:02:40 PST
Comment on attachment 115349 [details] Patch This seems like a bad way to factor this. You are adding a function to share code but very little is actually used in the other case.
Roland Steiner
Comment 5 2011-11-16 17:46:20 PST
FWIW, I completely agree! :P Originally the function only took a collectRulesOnly parameter that was set to false for page rules. See the patch for 67720 for the original version. FWIW, no CSS test breaks with this, even with the link style twiddling added, but since matchPageRules bypasses the twiddling it felt dangerous to re-use that branch in the refactoring.
Roland Steiner
Comment 6 2011-11-21 17:47:33 PST
Given the code changes, this refactoring is probably meaningless without the context of the larger patch after all. Marking INVALID.
Note You need to log in before you can comment on or make changes to this bug.