AncestorChainWalker follows the style of ComposedShadowTreeWalker, which also breaks conventions like mutations-should-be-verbs. We also fix its style for integrity.
Non-trivial tree traversal API in general should, IMO, be modeled after recently introduced NodeTraversal. Having separate namespaces allows us to keep Node API minimal. Also it is stateless and simple. I made NodeRenderingTraversal in that way. AncestorChainWalker could be just merged to NodeRenderingTraversal. Speaking of ComposedShadowTreeWalker, I think it is worth having a class instead of namespace since it holds configuration parameters to decide, for example, whether it skips specific types of node. On Tue, Feb 19, 2013 at 10:37 AM, Darin Adler <da...@apple.com> wrote: > I am trying to learn about code related to shadow DOM that needs to be > understood by anyone working on an part of the DOM. > > I have some questions and concerns about AncestorChainWalker. If there is > a document I should be reading that covers these, please point me to it. > > The parentNode and parentElement functions are just plain old functions > that are used to walk the ancestor chain of the normal DOM. But the shadow > DOM AncestorChainWalker is a class. Why? It seems that it could just be a > function with two return values. Is it more efficient to have it be a > class? Can the operation be correctly done without storing > m_distributedNode in a data member? > > There is a function in AncestorChainWalker named parent. That name is a > noun, so the function should be a const function that returns a value. > Since it’s not, the function name should be a verb phrase, such as > advanceToParent, or event just “advance” since it’s in the context of an > ancestor chain walker. > > The function crossingInsertionPoint should be named > isCrossingInsertionPoint as the data member is. But also, since the walker > sits still on a single node, I don’t think it makes sense to talk about the > position as “is crossing”. It should be “just crossed” or something like > that instead. Unless an insertion point is like a bridge and is not itself > a “true node”. > > The function that returns the current node in the ancestor chain is named > “get”. That’s not a good name, and should be avoided if possible. It could > be named “node” or “currentNode” instead. > > -- Darin > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
