Patch will follow.
Created attachment 49334 [details] [Chromium] Implement WebLabelElement
Attachment 49334 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/public/WebLabelElement.h:44: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit/chromium/public/WebLabelElement.h:49: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Re: style bot. This addition matches the style of WebInputElement.h, should I ignore the errors or fix them?
Created attachment 49346 [details] [Chromium] Implement WebLabelElement [try2] I decided to go ahead and fix the style errors.
Comment on attachment 49346 [details] [Chromium] Implement WebLabelElement [try2] > Index: WebKit/chromium/public/WebLabelElement.h > + WebLabelElement(const WebLabelElement& n) : WebElement(n) { } > + > + WebLabelElement& operator=(const WebLabelElement& n) > + { > + WebElement::assign(n); > + return *this; > + } > + > + WEBKIT_API void assign(const WebLabelElement& n) { WebElement::assign(n); } ^^^ nit: s/n/e/ perhaps? where "e" stands for element, but "n" doesn't seem to be related to the type name WebLabelElement. > +#if WEBKIT_IMPLEMENTATION > + WebLabelElement(const WTF::PassRefPtr<WebCore::HTMLLabelElement>&); > + WebLabelElement& operator=(const WTF::PassRefPtr<WebCore::HTMLLabelElement>&); > + operator WTF::PassRefPtr<WebCore::HTMLLabelElement>() const; > +#endif > + > + WEBKIT_API WebElement correspondingControl(); > +}; ^^^ nit: please put the WEBKIT_IMPLEMENTATION section at the end of the class to help make the actual public API a bit more visible. > Index: WebKit/chromium/src/WebLabelElement.cpp > +WebLabelElement::WebLabelElement(const WTF::PassRefPtr<HTMLLabelElement>& elem) > + : WebElement(elem) > +{ > +} > + > +WebLabelElement& WebLabelElement::operator=(const WTF::PassRefPtr<HTMLLabelElement>& elem) nit: no need to mention WTF:: since there is a global 'using namespace WTF'
Created attachment 49444 [details] [Chromium] Implement WebLabelElement [try3] Fixed nits.
Comment on attachment 49444 [details] [Chromium] Implement WebLabelElement [try3] > Index: WebKit/chromium/public/WebLabelElement.h > + WebLabelElement(const WebLabelElement& n) : WebElement(n) { } > + > + WebLabelElement& operator=(const WebLabelElement& n) > + { > + WebElement::assign(n); > + return *this; "n" -> "e" :) r=me, but that name should be fixed before committing. so if you could upload another patch for the commit-queue, that'd be great. thanks!
Created attachment 49448 [details] [Chromium] Implement WebDocument::getElementsByTagName [try4] Fixes remaining s/n/e/ nits that I missed in the last patch.
Comment on attachment 49448 [details] [Chromium] Implement WebDocument::getElementsByTagName [try4] Rejecting patch 49448 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Darin Fisher', '--force']" exit_code: 2 patching file WebKit/chromium/ChangeLog patch: **** unexpected end of file in patch patching file WebKit/chromium/WebKit.gyp patching file WebKit/chromium/public/WebLabelElement.h patching file WebKit/chromium/src/WebLabelElement.cpp Full output: http://webkit-commit-queue.appspot.com/results/309362
Created attachment 49508 [details] [Chromium] Implement WebLabelElement [try5] I edited the Changelog file itself this time instead of the patch...doh.
Comment on attachment 49508 [details] [Chromium] Implement WebLabelElement [try5] Clearing flags on attachment: 49508 Committed r55254: <http://trac.webkit.org/changeset/55254>
Not sure what the status is here. Is [try5] supposed to be reviewed and landed?
Yea, try5 was committed. Wasn't marked as fixed for some reason?
The commit-queue won't close bugs where other patches are marked r+/r?. I finally caved and added this "feature" to allow multi-patch bugs and after great protest from other users. :)
If folks use webkit-patch to upload their patches it will obsolete the previous patches. In general that's a good idea so as not to confuse reviewers by having multiple patches up for review or marked r+, etc.