Bug 35272

Summary:

[Chromium] Implement WebDocument::getElementsByTagName

Product:

WebKit

Reporter:

James Hawkins <jhawkins>

Component:

WebKit API

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dglazkov, fishd, skyul, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
[Chromium] Implement WebDocument::getElementsByTagName	none
[Chromium] Implement WebDocument::getElementsByTagName [try2]	fishd: review-
[Chromium] Implement WebNode::getElementsByTagName [try2]	none
[Chromium] Implement WebDocument::getElementsByTagName [try3]	none

James Hawkins

Reported 2010-02-22 16:44:18 PST

Patch will follow.

Attachments
[Chromium] Implement WebDocument::getElementsByTagName (1.93 KB, patch) 2010-02-22 16:45 PST, James Hawkins	no flags	Details Formatted Diff Diff
[Chromium] Implement WebDocument::getElementsByTagName [try2] (2.11 KB, patch) 2010-02-22 16:53 PST, James Hawkins	fishd: review-	Details Formatted Diff Diff
[Chromium] Implement WebNode::getElementsByTagName [try2] (1.69 KB, patch) 2010-02-23 11:02 PST, James Hawkins	no flags	Details Formatted Diff Diff
[Chromium] Implement WebDocument::getElementsByTagName [try3] (2.11 KB, patch) 2010-02-23 13:36 PST, James Hawkins	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

James Hawkins

Comment 1 2010-02-22 16:45:29 PST

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

WebKit Review Bot

Comment 2 2010-02-22 16:51:55 PST

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

James Hawkins

Comment 3 2010-02-22 16:53:12 PST

Created attachment 49248 [details] [Chromium] Implement WebDocument::getElementsByTagName [try2] Added missing include.

Darin Fisher (:fishd, Google)

Comment 4 2010-02-22 22:12:40 PST

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

Comment 5 2010-02-23 10:52:06 PST

(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

Comment 6 2010-02-23 11:02:09 PST

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

Darin Fisher (:fishd, Google)

Comment 7 2010-02-23 13:35:26 PST

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

Comment 8 2010-02-23 13:36:34 PST

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 9 2010-02-23 14:17:07 PST

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

Comment 10 2010-02-23 14:17:13 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.