Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(329)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 119063002: Have HTMLFormElement's named getter return a RadioNodeList. (Closed)

Created:
7 years ago by sof
Modified:
7 years ago
Reviewers:
tkent, esprehn, kojih
CC:
blink-reviews, arv+blink, eae+blinkwatch, dglazkov+blink, adamk+blink_chromium.org, watchdog-blink-watchlist_google.com, Inactive
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Have HTMLFormElement's named getter return a RadioNodeList. The getter for HTMLFormElement is now required to return a RadioNodeList if multiple form elements match, http://www.whatwg.org/specs/web-apps/current-work/#dom-form-nameditem Provide this, with the extra wrinkle of having to support a live RadioNodeList over only <img> elements if no 'listed elements' match the given ID/name. R=esprehn@chromium.org,tkent@chromium.org BUG=315851 TEST=fast/forms/form-radio-node-list TEST=fast/forms/form-radio-img-node-list Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164232

Patch Set 1 #

Patch Set 2 : Update expected output for inspector/console/console-format-collections #

Total comments: 14

Patch Set 3 : Add a RadioImgNodeListType CollectionType and use it + test improvements. #

Total comments: 12

Patch Set 4 : Test style adjustments + const'ify new RadioNodeList field. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+175 lines, -22 lines) Patch
A LayoutTests/fast/forms/form-radio-img-node-list.html View 1 2 3 1 chunk +46 lines, -0 lines 0 comments Download
A LayoutTests/fast/forms/form-radio-img-node-list-expected.txt View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download
A LayoutTests/fast/forms/form-radio-node-list.html View 1 2 3 1 chunk +42 lines, -0 lines 0 comments Download
A LayoutTests/fast/forms/form-radio-node-list-expected.txt View 1 2 3 1 chunk +27 lines, -0 lines 0 comments Download
M LayoutTests/inspector/console/console-format-collections.html View 1 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/inspector/console/console-format-collections-expected.txt View 1 4 chunks +4 lines, -4 lines 0 comments Download
M Source/core/dom/Node.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/dom/Node.cpp View 1 2 1 chunk +4 lines, -3 lines 0 comments Download
M Source/core/html/CollectionType.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/HTMLCollection.cpp View 1 2 4 chunks +4 lines, -0 lines 0 comments Download
M Source/core/html/HTMLFormElement.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLFormElement.cpp View 4 chunks +4 lines, -3 lines 0 comments Download
M Source/core/html/HTMLFormElement.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/RadioNodeList.h View 1 2 3 2 chunks +6 lines, -4 lines 0 comments Download
M Source/core/html/RadioNodeList.cpp View 1 2 6 chunks +13 lines, -4 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
sof
Please take a look when you next have a spare moment.
7 years ago (2013-12-19 10:37:44 UTC) #1
esprehn
https://codereview.chromium.org/119063002/diff/20001/LayoutTests/fast/forms/form-collection-radio-node-list.html File LayoutTests/fast/forms/form-collection-radio-node-list.html (right): https://codereview.chromium.org/119063002/diff/20001/LayoutTests/fast/forms/form-collection-radio-node-list.html#newcode126 LayoutTests/fast/forms/form-collection-radio-node-list.html:126: resetElement.id = "commoninput"; I'd rather you just added a ...
7 years ago (2013-12-19 11:02:48 UTC) #2
sof
On 2013/12/19 11:02:48, esprehn wrote: .. > > https://codereview.chromium.org/119063002/diff/20001/Source/core/html/RadioNodeList.h#newcode61 > Source/core/html/RadioNodeList.h:61: public: > I'd rather ...
7 years ago (2013-12-19 11:09:04 UTC) #3
sof
Good stuff, thanks for the careful reviewing. https://codereview.chromium.org/119063002/diff/20001/LayoutTests/fast/forms/form-collection-radio-node-list.html File LayoutTests/fast/forms/form-collection-radio-node-list.html (right): https://codereview.chromium.org/119063002/diff/20001/LayoutTests/fast/forms/form-collection-radio-node-list.html#newcode126 LayoutTests/fast/forms/form-collection-radio-node-list.html:126: resetElement.id = ...
7 years ago (2013-12-19 12:15:04 UTC) #4
tkent
lgtm https://codereview.chromium.org/119063002/diff/40001/LayoutTests/fast/forms/form-radio-img-node-list.html File LayoutTests/fast/forms/form-radio-img-node-list.html (right): https://codereview.chromium.org/119063002/diff/40001/LayoutTests/fast/forms/form-radio-img-node-list.html#newcode47 LayoutTests/fast/forms/form-radio-img-node-list.html:47: <p id="description"></p> nit: You don't need to write ...
7 years ago (2013-12-20 00:18:11 UTC) #5
sof
https://codereview.chromium.org/119063002/diff/40001/LayoutTests/fast/forms/form-radio-img-node-list.html File LayoutTests/fast/forms/form-radio-img-node-list.html (right): https://codereview.chromium.org/119063002/diff/40001/LayoutTests/fast/forms/form-radio-img-node-list.html#newcode47 LayoutTests/fast/forms/form-radio-img-node-list.html:47: <p id="description"></p> On 2013/12/20 00:18:11, tkent wrote: > nit: ...
7 years ago (2013-12-20 07:02:12 UTC) #6
sof
Landing this CL. If we want to slightly adjust the representation of RadioNodeList kinds, I'll ...
7 years ago (2013-12-20 11:26:26 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/119063002/60001
7 years ago (2013-12-20 11:26:52 UTC) #8
commit-bot: I haz the power
7 years ago (2013-12-20 13:08:45 UTC) #9
Message was sent while issue was closed. 
Change committed as 164232

Powered by Google App Engine
This is Rietveld 408576698