This is the last remaining client of Traversal<> outside the iterator implementation.
Created attachment 220705 [details] patch
Comment on attachment 220705 [details] patch Very snug.
https://trac.webkit.org/r161551
Comment on attachment 220705 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=220705&action=review > Source/WebCore/dom/ElementChildIterator.h:160 > + if (!isElementOfType<const ElementType>(child)) > + return end(); If this element is a child of m_parent, just not the correct type, I would expect this function to return the next child of the correct type if any, not always end(). A way to avoid this problem entirely would be to make the argument be ElementType& instead of const Element&.
Comment on attachment 220705 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=220705&action=review >> Source/WebCore/dom/ElementChildIterator.h:160 >> + return end(); > > If this element is a child of m_parent, just not the correct type, I would expect this function to return the next child of the correct type if any, not always end(). > > A way to avoid this problem entirely would be to make the argument be ElementType& instead of const Element&. I guess I’m wrong. We are using end() here as a sort of null value. But I do think that we should have a find that takes the specific element type to avoid the runtime isElementOfType check.
Comment on attachment 220705 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=220705&action=review >>> Source/WebCore/dom/ElementChildIterator.h:160 >>> + return end(); >> >> If this element is a child of m_parent, just not the correct type, I would expect this function to return the next child of the correct type if any, not always end(). >> >> A way to avoid this problem entirely would be to make the argument be ElementType& instead of const Element&. > > I guess I’m wrong. We are using end() here as a sort of null value. > > But I do think that we should have a find that takes the specific element type to avoid the runtime isElementOfType check. I guess I really don’t like find for the name of this. It sounds like something that iterates the children, when really it’s just a sort of alternate iterator constructor.
(In reply to comment #6) > I guess I really don’t like find for the name of this. It sounds like something that iterates the children, when really it’s just a sort of alternate iterator constructor. I mostly added it because we had find like this already in ElementDescendantIteratorAdapter. I agree that it is a silly name and should be replaced with a constructor function. I don't want to call constructors explicitly from regular code though. Maybe FooAdapter::iteratorFor(ElementType&)?