[Webkit-unassigned] [Bug 89061] applyAuthorStyles makes rules declared in all enclosing shadow dom subtrees applicable.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=89061





--- Comment #5 from Takashi Sakamoto <tasak at google.com>  2012-06-14 21:46:11 PST ---

(In reply to comment #4)
> (From update of attachment 147506 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147506&action=review
> 
> > Source/WebCore/css/StyleResolver.cpp:945
> > +        for (const ContainerNode* scope = m_element; scopedIndex > authorRuleIndex && scope; scope = scope->parentOrHostNode()) {
> 
> in this context, authorRuleIndex means first shadow DOM scope. It sounds like moving this code to a static non-class function would serve well to better explain this.
> 
> Why are we going up using parentOrHostNode?
> 
> > Source/WebCore/css/StyleResolver.cpp:948
> > +            for (; scopedIndex > authorRuleIndex; --scopedIndex)
> > +                if (m_scopeStack[scopedIndex - 1].m_scope != scope)
> > +                    break;
> 
> Can you explain what this code is doing? It seems to me that just going up the scope stack should be enough?

Because m_scopeStack doesn't have all ancestor nodes of m_element. m_scopeStack has information like (node, ruleset) only if the node has some rule. For examples,

Document
  A
  |
  B ---- ShadowRoot1
                 |
                 C---------ShadowRoot2 (applyAuthorStyles=true)
                                           |
                                          D---------- ShadowRoot3 (applyAuthorStyles=true)
                                                                      |
                                                                     E

and suppose that ShadowRoot1 has some <style> and that  C, D, E, ShadowRoot2, and ShadowRoot3 have no <style>.

When matchScopedAuthorRules is invoked and m_element is E, m_scopeStack looks like:
[(ShadowRoot1, some RuleSet)]
ShadowRoot2, D, ShadowRoot3, E are not stored in m_scopeStack.

So, by going up using parentOrHostNode(), generate the following sequence:
[E, ShadowRoot3, D, ShadowRoot2, C, ShadowRoot1]
and compare with m_scopeStack:

E: not in m_scopeStack, skip.
ShadowRoot3: not in m_scopeStack, skip. Since ShadowRoot3 has applyAuthorStyles set, continue.
D: not in m_scopeStack, skip.
ShadowRoot2: not in m_scopeStack, skip. Since ShadowRoot2 has applyAuthorStyles set, continue.
C: not in m_scopeStack, skip.
ShadowRoot1: in m_scopeStack. Try to apply.

It is difficult to find whether ShadowRoot2 and ShadowRoot3 have applyAuthorStyle set or not by just scanning m_scopeStack. 
This is also the reason why going up using parentOrHostNode().

However, I got an advice from Morita-san. I will change m_scopeStack to hold some information about applyAuthorStyles and update the patch.

> > LayoutTests/fast/css/style-scoped/style-scoped-apply-author-styles.html:32
> > +    document.body.offsetLeft;
> 
> I am worried that you keep using this to trigger style recalc. Shouldn't setting applyAuthorStyles trigger the recalc?

Yes, I'm using this to trigger style recalc… 
However, applyAuthorStyles should trigger recalc and the test should work without any "document.body.offsetLeft" for style realc.
I will remove these document.body.offsetLeft.

Best regards,
Takashi Sakamoto

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.