Bug 35272 – [Chromium] Implement WebDocument::getElementsByTagName

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[James Hawkins]

Patch will follow.

[James Hawkins]

Created attachment 49247 [details]
[Chromium] Implement WebDocument::getElementsByTagName

[WebKit Review Bot]

Attachment 49247 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/299294

[James Hawkins]

Created attachment 49248 [details]
[Chromium] Implement WebDocument::getElementsByTagName [try2]

Added missing include.

[Darin Fisher (:fishd, Google)]

Comment on attachment 49248 [details]
[Chromium] Implement WebDocument::getElementsByTagName [try2]

> Index: WebKit/chromium/public/WebDocument.h
...
>      WEBKIT_API WebElement getElementById(const WebString& id) const;
> +    WEBKIT_API WebNodeList getElementsByTagName(const WebString&);

hmm, it seems like getElementsByTagName (like getElementById) should be
a const method since it does not mutate WebDocument.

[James Hawkins]

(In reply to comment #4)
> (From update of attachment 49248 [details])
> > Index: WebKit/chromium/public/WebDocument.h
> ...
> >      WEBKIT_API WebElement getElementById(const WebString& id) const;
> > +    WEBKIT_API WebNodeList getElementsByTagName(const WebString&);
> 
> hmm, it seems like getElementsByTagName (like getElementById) should be
> a const method since it does not mutate WebDocument.

More importantly, getElementsByTagName belongs in WebNode, not WebDocument.  The original problem remains though: I can't make WebNode::getElementsByTagName const because WebCore::Node::getElementsByTagName is non-const.  The Node method caches the NodeList in the document for future reference.  Patch will follow.

[James Hawkins]

Created attachment 49305 [details]
[Chromium] Implement WebNode::getElementsByTagName [try2]

[Darin Fisher (:fishd, Google)]

Ah, yeah... it does return a live node list.  OK.  Anyways, the constness stuff is a little wacky since we are not returning a list of const WebNodes.  One thing, I think you should keep getElementsByTagName on WebDocument to align with Document.idl.  I'd consider it an implementation detail of WebCore that getElementsByTagName happens to be defined on class Node.

[James Hawkins]

Created attachment 49323 [details]
[Chromium] Implement WebDocument::getElementsByTagName [try3]

Moved the implementation back to WebDocument.  This is a copy of attachment 49248 [details].  That comment about non-const still applies.

[WebKit Commit Bot]

Comment on attachment 49323 [details]
[Chromium] Implement WebDocument::getElementsByTagName [try3]

Clearing flags on attachment: 49323

Committed r55171: <http://trac.webkit.org/changeset/55171>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.