Index setter of HTMLSelectElement and HTMLOptionsCollection should do nothing if the requested size…

Index setters of HTMLSelectElement and HTMLOptionsCollection should do nothing if the requested index is too large.

They should be consistent with their |length| setters.

Also, The index setters and the |length| setters have two behavior
changes as follows:

  * Do not skip processing if an option list has enough size.
    We don't limit an option list length if the options were parsed by the
    HTML parser.  So an option list length can be larger than maxSelectItems.
    There's no reason to skip the processing or shrink the length if
    the list already has enough size.

  * Show console warnings when they do nothing due to the length limitation.

BUG=522802
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200893

[tkent, 2015-08-20 07:25:41]

tkent@chromium.org changed reviewers:
+ davve@opera.com, keishi@chromium.org

[tkent, 2015-08-20 07:25:42]

Keishi, David, please review this.

Context: https://codereview.chromium.org/1292653003

[keishi, 2015-08-20 07:39:46]

LGTM

https://codereview.chromium.org/1291263003/diff/1/Source/core/html/HTMLSelect...
File Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/1291263003/diff/1/Source/core/html/HTMLSelect...
Source/core/html/HTMLSelectElement.cpp:473: String::format("A request to update
an option at index=%u is ignored.  The maximum length is %u.", index,
maxSelectItems)));
nit: Other messages seemed to be like "Failed to blah blah", "Blocked blah
blah". Maybe it would be more consistent if the subject was the UserAgent.

[davve, 2015-08-20 07:40:43]

LGTM

https://codereview.chromium.org/1291263003/diff/1/Source/core/html/HTMLSelect...
File Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/1291263003/diff/1/Source/core/html/HTMLSelect...
Source/core/html/HTMLSelectElement.cpp:471: if (index >= length() && index >=
maxSelectItems) {
I like that you added the comparison to current length()!

That means that a parser-generated list with more than maxSelectItems (10000)
will not be silently clamped when using these setters.

Does this change in behavior warrant a note in the description perhaps?

[tkent, 2015-08-20 08:05:47]

https://codereview.chromium.org/1291263003/diff/1/Source/core/html/HTMLSelect...
File Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/1291263003/diff/1/Source/core/html/HTMLSelect...
Source/core/html/HTMLSelectElement.cpp:471: if (index >= length() && index >=
maxSelectItems) {
On 2015/08/20 07:40:43, David Vest wrote:
> I like that you added the comparison to current length()!
> 
> That means that a parser-generated list with more than maxSelectItems (10000)
> will not be silently clamped when using these setters.
> 
> Does this change in behavior warrant a note in the description perhaps?

Yeah, I'll add this to the description.
Shrinking the existing options is not reasonable.  IMO, this 10,000 limitation
itself doesn't make much sense.

https://codereview.chromium.org/1291263003/diff/1/Source/core/html/HTMLSelect...
Source/core/html/HTMLSelectElement.cpp:473: String::format("A request to update
an option at index=%u is ignored.  The maximum length is %u.", index,
maxSelectItems)));
On 2015/08/20 07:39:46, keishi wrote:
> nit: Other messages seemed to be like "Failed to blah blah", "Blocked blah
> blah". Maybe it would be more consistent if the subject was the UserAgent.

will do.

[tkent, 2015-08-20 09:17:32]

The CQ bit was checked by tkent@chromium.org

[tkent, 2015-08-20 09:17:33]

The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org,
davve@opera.com
Link to the patchset: https://codereview.chromium.org/1291263003/#ps20001
(title: "Update warning messages")

[commit-bot: I haz the power, 2015-08-20 09:17:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1291263003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1291263003/20001

[commit-bot: I haz the power, 2015-08-20 10:26:02]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200893