Setting Proper values to HTMLOptionElement DirtyFlag

Form reset should clear the dirty flag of HTMLOptionElement

HTMLOptionElement::SetDirty() ignored the argument.  This CL fixes it.


Signed-off-by: sujiths.s <sujiths.s@samsung.com>;
Review-Url: https://codereview.chromium.org/2906883002
Cr-Commit-Position: refs/heads/master@{#476128}
Committed: https://chromium.googlesource.com/chromium/src/+/6d095ec7a71eb72e2ad9992d1ca3080f5726f6bf

[sujith, 2017-05-26 13:19:21]

sujiths.s@samsung.com changed reviewers:
+ kbr@chromium.org, ojan@chromium.org, tkent@chromium.org

[sujith, 2017-05-26 13:19:21]

PTAL

[tkent, 2017-05-26 16:19:22]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-26 16:19:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-26 18:11:26]

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

[commit-bot: I haz the power, 2017-05-26 18:11:27]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[ojan, 2017-05-27 19:51:52]

Please add test coverage for this. Does this cause an existing test to pass?

On Fri, May 26, 2017 at 9:19 AM <sujiths.s@samsung.com> wrote:

> Reviewers: tkent, Ken Russell, ojan
> CL: https://codereview.chromium.org/2906883002/
>
> Message:
> PTAL
>
> Description:
> Setting Proper values to HTMLOptionElement DirtyFlag
>
> dirty flag was hardcoded, changed it to proper value.
>
> BUG=723003
>
> Signed-off-by: sujiths.s <sujiths.s@samsung.com&gt;
>
> Affected files (+1, -1 lines):
> M third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
>
>
> Index: third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> diff --git a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> index
>
e28f864e91500a3708e770445b4de74d6db49d60..6f0ec4f152b67d7b6681511f42ff8cefb687ecfa
> 100644
> --- a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> +++ b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> @@ -284,7 +284,7 @@ void HTMLOptionElement::SetSelectedState(bool
> selected) {
> }
>
> void HTMLOptionElement::SetDirty(bool value) {
> - is_dirty_ = true;
> + is_dirty_ = value;
> }
>
> void HTMLOptionElement::ChildrenChanged(const ChildrenChange& change) {
>
>
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[ojan, 2017-05-27 19:51:53]

Please add test coverage for this. Does this cause an existing test to pass?

On Fri, May 26, 2017 at 9:19 AM <sujiths.s@samsung.com> wrote:

> Reviewers: tkent, Ken Russell, ojan
> CL: https://codereview.chromium.org/2906883002/
>
> Message:
> PTAL
>
> Description:
> Setting Proper values to HTMLOptionElement DirtyFlag
>
> dirty flag was hardcoded, changed it to proper value.
>
> BUG=723003
>
> Signed-off-by: sujiths.s <sujiths.s@samsung.com&gt;
>
> Affected files (+1, -1 lines):
> M third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
>
>
> Index: third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> diff --git a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> index
>
e28f864e91500a3708e770445b4de74d6db49d60..6f0ec4f152b67d7b6681511f42ff8cefb687ecfa
> 100644
> --- a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> +++ b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> @@ -284,7 +284,7 @@ void HTMLOptionElement::SetSelectedState(bool
> selected) {
> }
>
> void HTMLOptionElement::SetDirty(bool value) {
> - is_dirty_ = true;
> + is_dirty_ = value;
> }
>
> void HTMLOptionElement::ChildrenChanged(const ChildrenChange& change) {
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sujith, 2017-05-29 14:55:45]

I think already coverage test case is there
src/third_party/WebKit/LayoutTests/external/wpt/html/semantics/forms/resetting-a-form/reset-form-2.html
Checking for the build failure, since our test page
http://w3c-test.org/html/semantics/forms/resetting-a-form/reset-form-2.html
and the build failure case is same. 
I will update ASAP.



On 2017/05/27 19:51:53, ojan wrote:
> Please add test coverage for this. Does this cause an existing test to pass?
> 
> On Fri, May 26, 2017 at 9:19 AM <mailto:sujiths.s@samsung.com> wrote:
> 
> > Reviewers: tkent, Ken Russell, ojan
> > CL: https://codereview.chromium.org/2906883002/
> >
> > Message:
> > PTAL
> >
> > Description:
> > Setting Proper values to HTMLOptionElement DirtyFlag
> >
> > dirty flag was hardcoded, changed it to proper value.
> >
> > BUG=723003
> >
> > Signed-off-by: sujiths.s <mailto:sujiths.s@samsung.com&gt;
> >
> > Affected files (+1, -1 lines):
> > M third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> >
> >
> > Index: third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> > diff --git a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> > b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> > index
> >
>
e28f864e91500a3708e770445b4de74d6db49d60..6f0ec4f152b67d7b6681511f42ff8cefb687ecfa
> > 100644
> > --- a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> > +++ b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
> > @@ -284,7 +284,7 @@ void HTMLOptionElement::SetSelectedState(bool
> > selected) {
> > }
> >
> > void HTMLOptionElement::SetDirty(bool value) {
> > - is_dirty_ = true;
> > + is_dirty_ = value;
> > }
> >
> > void HTMLOptionElement::ChildrenChanged(const ChildrenChange& change) {
> >
> >
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[sujith, 2017-05-30 15:11:53]

@ojan, @tkent, 
When I executed the LayoutTest locally, this patch cause an
existing test case to pass(forms/resetting-a-form/reset-form-2.html).

But in the build bot it shows a failure at linux_chromium_rel_ng,
win_chromium_rel_ng
and mac_chromium_rel_ng. I am not sure why its failing. 
Could you please help me to identify why its failing in these tests

[ojan, 2017-05-30 21:52:39]

You can get the diff by looking at the layout_test_results link on the
buildbot page. In this case, it looks like you need to rebaseline the
expected test output. If you run the test using the test runner instead of
just loading it in a page, you'll see the same failure that you see on the
bot. You can get to this page by searching google for "running blink layout
tests" to see how to run the tests.
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/layout_t...

On Tue, May 30, 2017 at 8:11 AM <sujiths.s@samsung.com> wrote:

> @ojan, @tkent,
> When I executed the LayoutTest locally, this patch cause an
> existing test case to pass(forms/resetting-a-form/reset-form-2.html).
>
> But in the build bot it shows a failure at linux_chromium_rel_ng,
> win_chromium_rel_ng
> and mac_chromium_rel_ng. I am not sure why its failing.
> Could you please help me to identify why its failing in these tests
>
> https://codereview.chromium.org/2906883002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[ojan, 2017-05-30 21:52:39]

You can get the diff by looking at the layout_test_results link on the
buildbot page. In this case, it looks like you need to rebaseline the
expected test output. If you run the test using the test runner instead of
just loading it in a page, you'll see the same failure that you see on the
bot. You can get to this page by searching google for "running blink layout
tests" to see how to run the tests.
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/layout_t...

On Tue, May 30, 2017 at 8:11 AM <sujiths.s@samsung.com> wrote:

> @ojan, @tkent,
> When I executed the LayoutTest locally, this patch cause an
> existing test case to pass(forms/resetting-a-form/reset-form-2.html).
>
> But in the build bot it shows a failure at linux_chromium_rel_ng,
> win_chromium_rel_ng
> and mac_chromium_rel_ng. I am not sure why its failing.
> Could you please help me to identify why its failing in these tests
>
> https://codereview.chromium.org/2906883002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sujith, 2017-05-31 10:25:21]

sujiths.s@samsung.com changed reviewers:
+ shanmuga.m@samsung.com

[sujith, 2017-05-31 10:25:21]

Hi All,
PTAL

[sujith, 2017-05-31 10:52:33]

The CQ bit was checked by sujiths.s@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-31 10:52:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 10:52:40]

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

[commit-bot: I haz the power, 2017-05-31 10:52:40]

Dry run: No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[tkent, 2017-05-31 23:14:53]

lgtm

CL description:
> Setting Proper values to HTMLOptionElement DirtyFlag

Please do not capitalize random words.  This should be
  Setting proper values to HTMLOptionElement's dirty flag.

Also, describing the behavior change is better for the summary:
  Form reset should clear the dirty flag of HTMLOptionElement

I'll update the CL description.

[tkent, 2017-05-31 23:15:12]

Description was changed from

==========
Setting Proper values to HTMLOptionElement DirtyFlag

dirty flag was hardcoded, changed it to proper value.

BUG=723003

Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

to

==========
Form reset should clear the dirty flag of HTMLOptionElement

dirty flag was hardcoded, changed it to proper value.

BUG=723003

Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

[tkent, 2017-05-31 23:15:57]

Description was changed from

==========
Form reset should clear the dirty flag of HTMLOptionElement

dirty flag was hardcoded, changed it to proper value.

BUG=723003

Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

to

==========
Form reset should clear the dirty flag of HTMLOptionElement

HTMLOptionElement::SetDirty() ignored the argument.  This CL fixes it.


BUG=723003
Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

[tkent, 2017-05-31 23:16:10]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2017-05-31 23:16:41]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 00:11:03]

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

[commit-bot: I haz the power, 2017-06-01 00:11:04]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tkent, 2017-06-01 00:13:50]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2017-06-01 00:14:39]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 01:18:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496276030784990,
"parent_rev": "09c6267d605eabfb49885e147f7de04ff2160641", "commit_rev":
"6d095ec7a71eb72e2ad9992d1ca3080f5726f6bf"}

[commit-bot: I haz the power, 2017-06-01 01:18:13]

Description was changed from

==========
Form reset should clear the dirty flag of HTMLOptionElement

HTMLOptionElement::SetDirty() ignored the argument.  This CL fixes it.


BUG=723003
Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

to

==========
Form reset should clear the dirty flag of HTMLOptionElement

HTMLOptionElement::SetDirty() ignored the argument.  This CL fixes it.


Signed-off-by: sujiths.s <sujiths.s@samsung.com>
Review-Url: https://codereview.chromium.org/2906883002
Cr-Commit-Position: refs/heads/master@{#476128}
Committed:
https://chromium.googlesource.com/chromium/src/+/6d095ec7a71eb72e2ad9992d1ca3...
==========

[commit-bot: I haz the power, 2017-06-01 01:18:15]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6d095ec7a71eb72e2ad9992d1ca3...