[webkit-dev] AncestorChainWalker substance and style thoughts

[Hajime Morrita]

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 <darin at 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 at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130219/08d6f5da/attachment.html>