HTMLOptionElement constructor arguments default values are not per spec

HTMLOptionElement constructor arguments default
values are not per spec

As per spec link:
https://html.spec.whatwg.org/#the-option-element,
default value for selected attribute is false.

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

[shiva.jm, 2015-06-24 06:45:46]

shiva.jm@samsung.com changed reviewers:
+ pdr@chromium.org, philipj@opera.com

[shiva.jm, 2015-06-24 06:46:07]

pls have a look.

[philipj_slow, 2015-06-24 09:08:56]

Thanks, this LGTM, but remove the text if some existing test covers it. If you
look at how the generated code changes, is there any way at all this change is
observable? I suspect that it is not.

https://codereview.chromium.org/1203943002/diff/1/LayoutTests/fast/dom/HTMLOp...
File LayoutTests/fast/dom/HTMLOptionElement/option-constructor.html (right):

https://codereview.chromium.org/1203943002/diff/1/LayoutTests/fast/dom/HTMLOp...
LayoutTests/fast/dom/HTMLOptionElement/option-constructor.html:2: <script
src="../../../resources/js-test.js"></script>
I think some of the existing tests should cover this, please grep for 'new
Option' to see if there's anything that isn't covered and if so modify an
existing test, perhaps one of:
fast/forms/option-constructor-selected.html
fast/js/custom-constructors.html

[shiva.jm, 2015-06-25 05:51:06]

Updated the review comments. pls have a look.

https://codereview.chromium.org/1203943002/diff/1/LayoutTests/fast/dom/HTMLOp...
File LayoutTests/fast/dom/HTMLOptionElement/option-constructor.html (right):

https://codereview.chromium.org/1203943002/diff/1/LayoutTests/fast/dom/HTMLOp...
LayoutTests/fast/dom/HTMLOptionElement/option-constructor.html:2: <script
src="../../../resources/js-test.js"></script>
On 2015/06/24 09:08:56, philipj wrote:
> I think some of the existing tests should cover this, please grep for 'new
> Option' to see if there's anything that isn't covered and if so modify an
> existing test, perhaps one of:
> fast/forms/option-constructor-selected.html
> fast/js/custom-constructors.html

Done.

[shiva.jm, 2015-06-25 05:51:42]

The CQ bit was checked by shiva.jm@samsung.com to run a CQ dry run

[shiva.jm, 2015-06-25 05:51:42]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1203943002/#ps20001
(title: " ")

[shiva.jm, 2015-06-25 06:01:05]

The CQ bit was unchecked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 06:01:23]

The CQ bit was checked by shiva.jm@samsung.com to run a CQ dry run

[shiva.jm, 2015-06-25 06:05:46]

The CQ bit was unchecked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 06:08:21]

The CQ bit was checked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 06:08:25]

The CQ bit was unchecked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 06:09:05]

The CQ bit was checked by shiva.jm@samsung.com to run a CQ dry run

[shiva.jm, 2015-06-25 07:02:52]

The CQ bit was checked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 07:02:52]

The CQ bit was unchecked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 07:03:11]

The CQ bit was checked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 07:03:11]

The CQ bit was unchecked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 07:03:40]

The CQ bit was checked by shiva.jm@samsung.com to run a CQ dry run

[shiva.jm, 2015-06-25 08:28:29]

The CQ bit was unchecked by shiva.jm@samsung.com

[shiva.jm, 2015-06-25 08:29:02]

The CQ bit was checked by shiva.jm@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2015-06-25 09:01:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1203943002/20001

[commit-bot: I haz the power, 2015-06-25 10:11:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-25 10:11:59]

Dry run: This issue passed the CQ dry run.

[shiva.jm, 2015-06-25 11:51:33]

On 2015/06/24 09:08:56, philipj wrote:
> Thanks, this LGTM, but remove the text if some existing test covers it. If you
> look at how the generated code changes, is there any way at all this change is
> observable? I suspect that it is not.
> 
>
https://codereview.chromium.org/1203943002/diff/1/LayoutTests/fast/dom/HTMLOp...
> File LayoutTests/fast/dom/HTMLOptionElement/option-constructor.html (right):
> 
>
https://codereview.chromium.org/1203943002/diff/1/LayoutTests/fast/dom/HTMLOp...
> LayoutTests/fast/dom/HTMLOptionElement/option-constructor.html:2: <script
> src="../../../resources/js-test.js"></script>
> I think some of the existing tests should cover this, please grep for 'new
> Option' to see if there's anything that isn't covered and if so modify an
> existing test, perhaps one of:
> fast/forms/option-constructor-selected.html
> fast/js/custom-constructors.html

Dry run has passed for patch set2, shall we commit this patch2?

[Habib Virji, 2015-06-25 13:19:30]

Apologise by mistake clicked on the closed button.

[Habib Virji, 2015-06-25 13:19:32]

Apologise by mistake clicked on the closed button.

[philipj_slow, 2015-06-26 14:28:38]

LGTM. Please reference a bug or remove the empty BUG= before sending to CQ.

https://codereview.chromium.org/1203943002/diff/20001/LayoutTests/fast/js/scr...
File LayoutTests/fast/js/script-tests/custom-constructors.js (right):

https://codereview.chromium.org/1203943002/diff/20001/LayoutTests/fast/js/scr...
LayoutTests/fast/js/script-tests/custom-constructors.js:46: shouldBeFalse("new
Option('somedata', 'somevalue', false, false).defaultSelected");
I don't think testing that the |selected| argument doesn't influence
|defaultSelected| is necessary, but the bots are happy now and it's just some
extra paranoia, so let's land this :)

[philipj_slow, 2015-06-26 14:28:55]

(Also wrap description to 72 columns.)

[philipj_slow, 2015-06-26 14:28:57]

(Also wrap description to 72 columns.)

[shiva.jm, 2015-06-26 15:49:04]

On 2015/06/26 14:28:38, philipj wrote:
> LGTM. Please reference a bug or remove the empty BUG= before sending to CQ.
> 
>
https://codereview.chromium.org/1203943002/diff/20001/LayoutTests/fast/js/scr...
> File LayoutTests/fast/js/script-tests/custom-constructors.js (right):
> 
>
https://codereview.chromium.org/1203943002/diff/20001/LayoutTests/fast/js/scr...
> LayoutTests/fast/js/script-tests/custom-constructors.js:46: shouldBeFalse("new
> Option('somedata', 'somevalue', false, false).defaultSelected");
> I don't think testing that the |selected| argument doesn't influence
> |defaultSelected| is necessary, but the bots are happy now and it's just some
> extra paranoia, so let's land this :)

Thanks Done.

[shiva.jm, 2015-06-26 15:49:18]

The CQ bit was checked by shiva.jm@samsung.com

[commit-bot: I haz the power, 2015-06-26 15:49:38]

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

[commit-bot: I haz the power, 2015-06-26 17:01:11]

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

[philipj_slow, 2015-07-01 11:40:16]

In this CL you accidentally removed the first line from the description. I've
made the same mistake, and it means that the resulting commit doesn't have a
good description.

[shiva.jm, 2015-07-01 11:56:59]

On 2015/07/01 11:40:16, philipj wrote:
> In this CL you accidentally removed the first line from the description. I've
> made the same mistake, and it means that the resulting commit doesn't have a
> good description.

I checked in the link:
http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLOptionElement...,
description/Log Message is visible fine. is the link i am looking is different
?.

[philipj_slow, 2015-07-01 14:43:59]

On 2015/07/01 11:56:59, shiva.jm wrote:
> On 2015/07/01 11:40:16, philipj wrote:
> > In this CL you accidentally removed the first line from the description.
I've
> > made the same mistake, and it means that the resulting commit doesn't have a
> > good description.
> 
> I checked in the link:
>
http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLOptionElement...,
> description/Log Message is visible fine. is the link i am looking is different
> ?.

Oh, I wasn't paying attention, now I see what happened. The title was just
wrapped to 72 columns (as I asked) and so it's split into two lines. No worries.

[shiva.jm, 2015-09-24 04:18:36]

On 2015/07/01 14:43:59, philipj wrote:
> On 2015/07/01 11:56:59, shiva.jm wrote:
> > On 2015/07/01 11:40:16, philipj wrote:
> > > In this CL you accidentally removed the first line from the description.
> I've
> > > made the same mistake, and it means that the resulting commit doesn't have
a
> > > good description.
> > 
> > I checked in the link:
> >
>
http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLOptionElement...,
> > description/Log Message is visible fine. is the link i am looking is
different
> > ?.
> 
> Oh, I wasn't paying attention, now I see what happened. The title was just
> wrapped to 72 columns (as I asked) and so it's split into two lines. No
worries.

These change was made to address FIX mes in HTML IDL files, these issue can be
associated to bug: https://code.google.com/p/chromium/issues/detail?id=460722.
may be its missed out in revision.