[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 #14 from Takashi Sakamoto <tasak at google.com>  2012-06-18 23:27:39 PST ---
(In reply to comment #12)
> (In reply to comment #11)
> > What I would like to do is grouping tree scopes by using applyAuthorStyles.
> 
> Oh I see! Then we have a different problem: the purpose of this member variable is very hard to figure out from its name. It's a Good Thing (tm) in WebKit to make sure that things are named appropriately -- so that when a future aspiring WebKit engineer looks at your code, they can understand its purpose quickly.

I agree that the name, scopeGroup, is not good. So I renamed it to applyAuthorStyleSetGroup in the new patch (2012-06-18's).

I'm also thinking of upper boundary of applicable shadom dom subtrees, but I think, as m_scopeStackParent's upper boundary should be updated when setupScopeStack or popScopeStack is invoked, so the methods will probably be modified in the following way:

void StyleResolver::setupScopeStack(…)
{
...
   const ContainerNode* firstBoundary = 0;
...
   for ( ; scope ; ) {
     const ContainerNode* boundary = 0;
     const ContainerNode* nextLowerBoundary = scope;
     for (nextLowerBoundary = scope; nextLowerBoundary; nextLowerBoundary = nextLowerBoundary->parentOrHostNode()) {
       if (isShadowRoot(nextLowerBoundary) && !toShadowRoot(nextLowerBoundary)->isShadowRoot()) {
          boundary = nextLowerBoundary;
          nextLowerBoundary = nextLowerBoundary->parentOrHostNode();
          break;
        }
     }
     if (!firstBoundary)
       firstBoudary = boundary; // 
     for ( ; scope != nextLowerBoundary; scope = scope->parentOrHostNode()) {
      …

     }
   }
   m_scopeStackParentUpperBoundary = firstBoundary;
   m_scopeParent = parent;
}

void StyleResolver::popScope(scope)
{
...
     if (isShadowRoot(scope) && !toShadowRoot(scope)->applyAuthorStyles()) {
         const ContainerNode* boudary = scope;
         // need to go up to find a new upper boundary.
         for (boudary = scope; boudary; boudary = boudary->parentOrHostNode())
           if (isShadowRoot(boudary) && !toShadowRoot(boudary)->isShadowRoot())
               break;
         m_scopeStackParentUpperBoundary = boundary;
     }
     m_scopeStackParent = scope->parentOrHostNode(); 
….


In this case, we can use m_applicableStyleUpperBoundary or something instead of m_scopeGroup. However I feel that the code is not as simple as the current patch.

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.