Expose checked field of HTMLInputElement to Chromium API
Created attachment 82226 [details] Patch
Comment on attachment 82226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82226&action=review > w/Source/WebKit/chromium/public/WebInputElement.h:81 > + WEBKIT_API bool checked() const; How about renaming this to isChecked to be more consistent with how other, similar API methods are named? "checked" is really trying to ask a question, so it helps to word it like a question.
Every other WebInputElement method is named identically to the HTMLInputElement method it calls though to. This would be the first WebInputElement method that uses a different name. Isn't consistency more important? Also, there's this comment in HTMLInputElement.h: // isChecked is used by the rendering tree/CSS while checked() is used by JS to determine checked state
(In reply to comment #3) > Every other WebInputElement method is named identically to the HTMLInputElement method it calls though to. This would be the first WebInputElement method that uses a different name. Isn't consistency more important? You're talking about an implementation detail. I'm arguing for making the publicly facing API consistent from the perspective of the guy who uses the API.
I will defer to your judgement, but in my experience abstractions leak and when that happens, it's less confusing for the same concepts at different layers to have the same name. Sending up new patch shortly.
Created attachment 82330 [details] Patch
Comment on attachment 82330 [details] Patch R=me It sounds like the internal naming is unfortunate. "isChecked" and "checked" sound way too similar. I'd recommend finding a better name for one of those.
The commit-queue encountered the following flaky tests while processing attachment 82330 [details]: fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: dumi@chromium.org) The commit-queue is continuing to process your patch.
Comment on attachment 82330 [details] Patch Clearing flags on attachment: 82330 Committed r78494: <http://trac.webkit.org/changeset/78494>
All reviewed patches have been landed. Closing bug.
(In reply to comment #7) > It sounds like the internal naming is unfortunate. "isChecked" and "checked" sound way too similar. I'd recommend finding a better name for one of those. I think we ended up with these names because isChecked is a good name and checked is the name used in the HTML DOM.
(In reply to comment #11) > (In reply to comment #7) > > It sounds like the internal naming is unfortunate. "isChecked" and "checked" sound way too similar. I'd recommend finding a better name for one of those. > > I think we ended up with these names because isChecked is a good name and checked is the name used in the HTML DOM. You don't find the names confusingly similar?
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #7) > > > It sounds like the internal naming is unfortunate. "isChecked" and "checked" sound way too similar. I'd recommend finding a better name for one of those. > > > > I think we ended up with these names because isChecked is a good name and checked is the name used in the HTML DOM. > > You don't find the names confusingly similar? I do. My intention was to explain how we got here, not defend where we are ;-) Itβs bad to have two similar names with different behavior. They should definitely be named in a way that makes the difference clear.
OK, I created a bug about the naming issue: see bug 54476.