[Webkit-unassigned] [Bug 79886] ShadowRoot should have activeElement

[bugzilla-daemon at webkit.org]

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


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #130076|review?                     |review-
               Flag|                            |




--- Comment #8 from Ryosuke Niwa <rniwa at webkit.org>  2012-03-05 16:06:31 PST ---
(From update of attachment 130076)
View in context: https://bugs.webkit.org/attachment.cgi?id=130076&action=review

> Source/WebCore/dom/TreeScope.cpp:167
> +Element* TreeScope::getActiveElement()

Please don't prefix this function's name by "get". The prefix "get" should be used for only functions that return values via out arguments.

> Source/WebCore/dom/TreeScope.cpp:178
> +        node = node->parentOrHostNode();

This is very inefficient way to walking up the tree. You should be able to skip all nodes between two tree scopes.
e.g.
Node* treeScope;
for (; node; node = treeScope->shadowHost()) {
    treeScope = node->treeScope();
    if (treeScope == this)
        return node;
}

> Source/WebCore/dom/TreeScope.cpp:183
> +    return document->body();

I don't think we should always be returning body element here.

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