100451 2012-10-25 21:01:22 -0700 [Shadow DOM][Meta] Changing attribute does not cause distribution. 2012-11-19 18:14:09 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Windows 7 RESOLVED FIXED P2 Normal --- 100738 100921 100922 101180 101697 101698 101699 101700 101891 101900 101901 101902 101903 102160 63606 1 sgrekhov webkit-unassigned dglazkov morrita shinyak webcomponents-bugzilla oldest_to_newest 751501 0 sgrekhov 2012-10-25 21:01:22 -0700 Found in Chrome 22.0.1229.94 m The following test (in fact rewritten Bob-developer example from Shadow DOM spec) fails: var iframe = document.createElement('iframe'); document.body.appendChild(iframe); iframe.contentDocument.body.innerHTML = "<ul class='stories'>" + "<li id='li1'><a href='#1'>Link1</a></li>" + "<li id='li2'><a href='#2'>Link 2</a></li>" + "<li id='li3' class='shadow'><a href='#3'>Link 3 Shadow</a></li>" + "<li id='li4'><a href='#4'>Link 4</a></li>" + "<li id='li5'><a href='#5'>Link 5</a></li>" + "<li id='li6' class='shadow'><a href='#5'>Link 6 Shadow</a></li>" + "</ul>"; var d = iframe.contentDocument; var ul = d.querySelector("ul.stories"); var SR = window.ShadowRoot || window.WebKitShadowRoot; var s = new SR(ul); //make shadow subtree var subdiv1 = document.createElement('div'); subdiv1.innerHTML = "<ul><content select='.shadow'></content></ul>"; s.appendChild(subdiv1); assert_true(d.querySelector('#li3').offsetTop < d.querySelector('#li6').offsetTop, 'Point 1: Elements that match insertion point criteria don\'t participate in distribution'); assert_true(d.querySelector('#li3').offsetTop > 0, 'Point 2: Elements that match insertion point criteria don\'t participate in distribution'); assert_equals(d.querySelector('#li1').offsetTop, 0, 'Point 3: Elements that don\'t match insertion point criteria participate in distribution'); assert_equals(d.querySelector('#li2').offsetTop, 0, 'Point 4: Elements that don\'t match insertion point criteria participate in distribution'); assert_equals(d.querySelector('#li4').offsetTop, 0, 'Point 5: Elements that don\'t match insertion point criteria participate in distribution'); assert_equals(d.querySelector('#li5').offsetTop, 0, 'Point 6: Elements that don\'t match insertion point criteria participate in distribution'); var li5 = d.querySelector('#li5'); li5.className = 'shadow'; assert_true(li5.offsetTop > 0, 'Distribution should reoccur if a variable affecting it is changed'); Class name of li5 was changed to the one that matches insertion point criteria, so the distribution should reoccur but it doesn't. 753932 1 shinyak 2012-10-29 18:49:58 -0700 I'll see it today. If it's found in chrome 22, we might have fixed it already though... 753974 2 shinyak 2012-10-29 20:55:28 -0700 OK. I've confirmed this test is still failing in the current trunk. 754111 3 shinyak 2012-10-30 02:26:02 -0700 Hmm... it seems changing attribute does not reflect immediately at all. Let's use this as a meta bug and fix them one by one. 754165 4 morrita 2012-10-30 03:42:11 -0700 (In reply to comment #3) > Hmm... it seems changing attribute does not reflect immediately at all. > Let's use this as a meta bug and fix them one by one. Right. we need to address this. On the other hand, it is bad idea to re-run distribution algorithm for each attribute change from a performance perspective. Can we do something more efficient like examining @select attribute values before invalidating the distribution? 754511 5 dglazkov 2012-10-30 10:54:20 -0700 (In reply to comment #4) > (In reply to comment #3) > > Hmm... it seems changing attribute does not reflect immediately at all. > > Let's use this as a meta bug and fix them one by one. > > Right. we need to address this. > On the other hand, it is bad idea to re-run distribution algorithm for each attribute change > from a performance perspective. It may sound pretty bad, but it's something CSS does anyway, right? > Can we do something more efficient like examining @select attribute values before > invalidating the distribution? This sounds like an interesting optimization: 1) determine effects of the select as some subset of DOM changes 2) act only on DOM changes that are in this subset. Also sounds: 1) incredibly complex 2) something that could be shared with general CSS rule matching 754849 6 shinyak 2012-10-30 18:16:16 -0700 (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > Hmm... it seems changing attribute does not reflect immediately at all. > > > Let's use this as a meta bug and fix them one by one. > > > > Right. we need to address this. > > On the other hand, it is bad idea to re-run distribution algorithm for each attribute change > > from a performance perspective. > > It may sound pretty bad, but it's something CSS does anyway, right? > > > Can we do something more efficient like examining @select attribute values before > > invalidating the distribution? > > This sounds like an interesting optimization: > 1) determine effects of the select as some subset of DOM changes > 2) act only on DOM changes that are in this subset. > > Also sounds: > 1) incredibly complex > 2) something that could be shared with general CSS rule matching Seeing Element::attributeChanged, Element checks CSS Selector to decide whether we should recalculate style. So maybe we can do this? It's complex though... 754931 7 shinyak 2012-10-30 21:14:31 -0700 I'm now thinking the following plan: 1. ShadowRoot should know the set of descendant InsertionPoint - Maybe we can hook insertedInto and removedFrom 2. ShadowRoot should also know the descendant elements having ElementShadow - We also track addShadowRoot() 3. ShadowRoot will have FeatureRuleSet or something like that - We update it when InsertionPoint / ElementShadow is added/removed 4. When attribute of the direct child (or non-direct child in fallback element case) is changed, we consult with the FeatureRuleSet whether the change is related or not. - If it's related, we invalidate the distribution. - If not, we recursively consult with ElementShadow. 754992 8 morrita 2012-10-30 23:12:38 -0700 (In reply to comment #7) > I'm now thinking the following plan: > > 1. ShadowRoot should know the set of descendant InsertionPoint > - Maybe we can hook insertedInto and removedFrom > 2. ShadowRoot should also know the descendant elements having ElementShadow > - We also track addShadowRoot() > 3. ShadowRoot will have FeatureRuleSet or something like that > - We update it when InsertionPoint / ElementShadow is added/removed > 4. When attribute of the direct child (or non-direct child in fallback element case) is changed, we consult with the FeatureRuleSet whether the change is related or not. > - If it's related, we invalidate the distribution. > - If not, we recursively consult with ElementShadow. Sounds like a plan. I hope we only need to track only the number of insertion points/shadows instead of the nodes themselves. Probably we can land the slow version fast, then introducing these optimisation. 755002 9 shinyak 2012-10-30 23:47:04 -0700 If we track InsertionPoint, distribution will be fast, because we can only iterate such elements instead of all descendant nodes. (This is not directly related to this bug, though.) So we might want consider it in another bug. 755007 10 shinyak 2012-10-30 23:49:49 -0700 I've created: https://bugs.webkit.org/show_bug.cgi?id=100820 764090 11 shinyak 2012-11-11 23:17:58 -0800 For pseudoClasses, please see this. http://www.whatwg.org/specs/web-apps/current-work/multipage/selectors.html#pseudo-classes 771341 12 shinyak 2012-11-19 18:14:09 -0800 All subbugs are closed. Let's close this.