Bug 32841 - [Chromium] Adding new methods to the Chromium DOM API
Summary: [Chromium] Adding new methods to the Chromium DOM API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Jay Campan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-21 14:50 PST by Jay Campan
Modified: 2010-01-04 17:12 PST (History)
8 users (show)

See Also:


Attachments
Adding some extra methods to the Chromium DOM API (needed for the translate feature) (4.77 KB, patch)
2009-12-21 14:50 PST, Jay Campan
no flags Details | Formatted Diff | Diff
45350: Adding some extra methods to the Chromium DOM API (needed for the translate feature) (4.19 KB, patch)
2009-12-22 09:18 PST, Jay Campan
fishd: review-
Details | Formatted Diff | Diff
Changed WebElement::getTagName() to WebElement::tagName() (4.16 KB, application/octet-stream)
2010-01-04 12:01 PST, Jay Campan
no flags Details
Changed WebElement::getTagName() to WebElement::tagName() (4.16 KB, patch)
2010-01-04 12:02 PST, Jay Campan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jay Campan 2009-12-21 14:50:17 PST
Created attachment 45350 [details]
Adding some extra methods to the Chromium DOM API (needed for the translate feature)

Adding new methods to the Chromium DOM API.
There are needed by the upcoming translate feature.
Comment 1 WebKit Review Bot 2009-12-21 14:50:42 PST
Attachment 45350 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/chromium/src/WebNode.cpp:90:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1
Comment 2 Eric Seidel (no email) 2009-12-21 21:49:04 PST
Darin or Dimitry is your best bet.  Also there is a style error in the patch.
Comment 3 Jay Campan 2009-12-22 09:18:21 PST
Created attachment 45389 [details]
45350: Adding some extra methods to the Chromium DOM API (needed for the translate feature)

Fixing style issue.
Comment 4 WebKit Review Bot 2009-12-22 09:21:25 PST
style-queue ran check-webkit-style on attachment 45389 [details] without any errors.
Comment 5 Adam Barth 2009-12-22 09:55:54 PST
Looks sane to me, but Darin is the arbiter of the API.
Comment 6 Darin Fisher (:fishd, Google) 2010-01-04 11:03:48 PST
Comment on attachment 45389 [details]
45350: Adding some extra methods to the Chromium DOM API (needed for the translate feature)

> Index: WebKit/chromium/src/WebElement.cpp
...
> +WebString WebElement::getTagName() const
> +{
> +    return constUnwrap<Element>()->tagName();

Please change WebElement::getTagName to just tagName.  WebKit
style avoids "get"

Otherwise, LGTM

-Darin
Comment 7 Jay Campan 2010-01-04 12:01:26 PST
Created attachment 45812 [details]
Changed WebElement::getTagName() to WebElement::tagName()
Comment 8 Jay Campan 2010-01-04 12:02:23 PST
Created attachment 45813 [details]
Changed WebElement::getTagName() to WebElement::tagName()
Comment 9 WebKit Commit Bot 2010-01-04 17:12:31 PST
Comment on attachment 45813 [details]
Changed WebElement::getTagName() to WebElement::tagName()

Clearing flags on attachment: 45813

Committed r52774: <http://trac.webkit.org/changeset/52774>
Comment 10 WebKit Commit Bot 2010-01-04 17:12:39 PST
All reviewed patches have been landed.  Closing bug.