Bug 101180

Summary: [Shadow] ElementShadow should have RuleFeatureSet for select attribute selectors.
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, gustavo, macpherson, menard, philn, webcomponents-bugzilla, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 101543, 101692    
Bug Blocks: 100451    
Attachments:
Description Flags
WIP
none
WIP
none
WIP
none
WIP
none
WIP
none
Patch none

Shinya Kawanaka
Reported 2012-11-04 22:18:55 PST
This is a preparation for https://bugs.webkit.org/show_bug.cgi?id=100451 When attribute is changed, we would like to consult with FeatureRuleSet whether we have to invalidate distribution or not.
Attachments
WIP (17.60 KB, patch)
2012-11-07 03:42 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
WIP (46.02 KB, patch)
2012-11-08 01:36 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
WIP (46.01 KB, patch)
2012-11-08 01:41 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
WIP (46.25 KB, patch)
2012-11-08 02:25 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
WIP (34.39 KB, patch)
2012-11-08 20:18 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (29.44 KB, patch)
2012-11-08 23:11 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2012-11-07 03:42:04 PST
Created attachment 172753 [details] WIP
Build Bot
Comment 2 2012-11-07 03:52:51 PST
Comment on attachment 172753 [details] WIP Attachment 172753 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14763153
Build Bot
Comment 3 2012-11-07 05:31:54 PST
Comment on attachment 172753 [details] WIP Attachment 172753 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14757360
Dimitri Glazkov (Google)
Comment 4 2012-11-07 09:04:03 PST
Comment on attachment 172753 [details] WIP Really excited about this direction. Being smart about when to redistribute is awesome.
Shinya Kawanaka
Comment 5 2012-11-08 01:36:15 PST
Created attachment 172957 [details] WIP
Shinya Kawanaka
Comment 6 2012-11-08 01:37:33 PST
This is almost reviewable, though it contains a patch for Bug 101543.
Shinya Kawanaka
Comment 7 2012-11-08 01:41:22 PST
Created attachment 172961 [details] WIP
Build Bot
Comment 8 2012-11-08 02:03:50 PST
Comment on attachment 172961 [details] WIP Attachment 172961 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14763482
Build Bot
Comment 9 2012-11-08 02:23:33 PST
Comment on attachment 172961 [details] WIP Attachment 172961 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14771088
Shinya Kawanaka
Comment 10 2012-11-08 02:25:29 PST
Created attachment 172967 [details] WIP
Build Bot
Comment 11 2012-11-08 05:34:08 PST
Comment on attachment 172967 [details] WIP Attachment 172967 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14757769
Dimitri Glazkov (Google)
Comment 12 2012-11-08 08:47:39 PST
Comment on attachment 172967 [details] WIP This looks great! May I suggest 3 patches? 1) refactor RuleFeature/RuleSet to expose collectFeaturesFromSelector 2) refactor HTMLContentElement to move in selector validation 3) everything else.
Shinya Kawanaka
Comment 13 2012-11-08 20:18:09 PST
Created attachment 173185 [details] WIP
Shinya Kawanaka
Comment 14 2012-11-08 20:19:19 PST
(In reply to comment #12) > (From update of attachment 172967 [details]) > This looks great! May I suggest 3 patches? > 1) refactor RuleFeature/RuleSet to expose collectFeaturesFromSelector > 2) refactor HTMLContentElement to move in selector validation > 3) everything else. Yes. (1) will be done in Bug 101692 (2) has been done in Bug 100738 (3) is here! The previous WIP patch contains (1) and (3).
Shinya Kawanaka
Comment 15 2012-11-08 23:06:21 PST
not Bug 100738 but Bug 101543.
Shinya Kawanaka
Comment 16 2012-11-08 23:11:16 PST
Created attachment 173213 [details] Patch
Dimitri Glazkov (Google)
Comment 17 2012-11-09 09:10:45 PST
Comment on attachment 173213 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173213&action=review > Source/WebCore/dom/ElementShadow.cpp:237 > + for (Node* node = root->firstChild(); node; node = node->traverseNextNode()) { Should this not only collect from children, not all descendants? > Source/WebCore/dom/ElementShadow.cpp:246 > + for (Node* node = root->firstChild(); node; node = node->traverseNextNode()) { Ditto.
Shinya Kawanaka
Comment 18 2012-11-10 00:27:16 PST
Comment on attachment 173213 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173213&action=review >> Source/WebCore/dom/ElementShadow.cpp:237 >> + for (Node* node = root->firstChild(); node; node = node->traverseNextNode()) { > > Should this not only collect from children, not all descendants? It should collect all descendants, right? I'm using traverseNextNode() instead of nextSibling()...
Shinya Kawanaka
Comment 19 2012-11-10 01:59:08 PST
(In reply to comment #18) > (From update of attachment 173213 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=173213&action=review > > >> Source/WebCore/dom/ElementShadow.cpp:237 > >> + for (Node* node = root->firstChild(); node; node = node->traverseNextNode()) { > > > > Should this not only collect from children, not all descendants? > > It should collect all descendants, right? > I'm using traverseNextNode() instead of nextSibling()... Ah, I see.. We don't need collect all descendants. Only direct children are required.
Shinya Kawanaka
Comment 20 2012-11-11 17:56:37 PST
Comment on attachment 173213 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173213&action=review >>>> Source/WebCore/dom/ElementShadow.cpp:237 >>>> + for (Node* node = root->firstChild(); node; node = node->traverseNextNode()) { >>> >>> Should this not only collect from children, not all descendants? >> >> It should collect all descendants, right? >> I'm using traverseNextNode() instead of nextSibling()... > > Ah, I see.. We don't need collect all descendants. Only direct children are required. Oops. This is not correct. We should collect all elements having ElementShadow. So this code should be right...
Shinya Kawanaka
Comment 21 2012-11-11 18:11:00 PST
Example: When className of div[class="foo"] is changed, we have to invalidate distribution. div -------------------------- ShadowRoot | - div[class="foo"] |- div ------------------------------ ShadowRoot |- div |- div ----- ShadowRoot |- shadow |-content[select=".foo"]
WebKit Review Bot
Comment 22 2012-11-11 19:40:35 PST
Comment on attachment 173213 [details] Patch Clearing flags on attachment: 173213 Committed r134184: <http://trac.webkit.org/changeset/134184>
WebKit Review Bot
Comment 23 2012-11-11 19:40:40 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.