Limit scope getter to predefined values

Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string (for default).

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope

Committed: https://crrev.com/f3f53a039bcdda5bfda56f1f5ac5fd6f84b3e704
Cr-Commit-Position: refs/heads/master@{#422144}

[rwlbuis, 2016-09-27 21:26:05]

Description was changed from

==========
scope WIP

BUG=
==========

to

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string as default.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

[rwlbuis, 2016-09-27 22:41:42]

Description was changed from

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string as default.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

to

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string as default.

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

[rwlbuis, 2016-09-27 22:42:15]

Description was changed from

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string as default.

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

to

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string (for default).

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

[rwlbuis, 2016-09-27 22:44:46]

rob.buis@samsung.com changed reviewers:
+ tkent@chromium.org

[rwlbuis, 2016-09-27 22:44:47]

PTAL.

BTW this also fixes the scope tests in:
http://w3c-test.org/html/dom/reflection-tabular.html

[tkent, 2016-09-28 00:23:08]

https://codereview.chromium.org/2376593003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp (right):

https://codereview.chromium.org/2376593003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp:178: if
(equalIgnoringCase(scopeValue, "row")) {
equalIgnoringCase should be equalIgnoringASCIICase.

https://html.spec.whatwg.org/multipage/infrastructure.html#enumerated-attribute

[rwlbuis, 2016-09-28 13:43:12]

https://codereview.chromium.org/2376593003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp (right):

https://codereview.chromium.org/2376593003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp:178: if
(equalIgnoringCase(scopeValue, "row")) {
On 2016/09/28 00:23:08, tkent wrote:
> equalIgnoringCase should be equalIgnoringASCIICase.
> 
>
https://html.spec.whatwg.org/multipage/infrastructure.html#enumerated-attribute

Done :)

[tkent, 2016-09-28 13:52:04]

Do we have tests for scope IDL getter?  e.g. IDL scope should return "row" for
scope="ROW".
reflection-tabular.html includes such test, but we don't import it and we need
test coverage.

[rwlbuis, 2016-09-28 14:06:27]

On 2016/09/28 13:52:04, tkent wrote:
> Do we have tests for scope IDL getter?  e.g. IDL scope should return "row" for
> scope="ROW".
> reflection-tabular.html includes such test, but we don't import it and we need
> test coverage.

I think dom/html/level2/html/table29.js comes closest but it has only one simple
test which we pass even before my patch. I guess more subtests could be added.

reflection-tabular.hml would have all the tests we need (the other reflection
test files should be useful too). I just wonder if there is any progress with
crbug.com/490939?

[tkent, 2016-09-29 02:28:16]

On 2016/09/28 at 14:06:27, rob.buis wrote:
> reflection-tabular.hml would have all the tests we need (the other reflection
test files should be useful too). I just wonder if there is any progress with
crbug.com/490939?

No progress.
I don't remember I tried LayoutTests/SlowTests or not.  If SlowTests resolves
the flakiness, we don't need to split tests.

If you'd like to land this soon, I recommend to add new tests (or copy a part of
reflection-tabular.html) to this CL, and add a comment about it to
crbug.com/490939.

[rwlbuis, 2016-09-29 13:43:08]

On 2016/09/29 02:28:16, tkent wrote:
> On 2016/09/28 at 14:06:27, rob.buis wrote:
> > reflection-tabular.hml would have all the tests we need (the other
reflection
> test files should be useful too). I just wonder if there is any progress with
> crbug.com/490939?
> 
> No progress.
> I don't remember I tried LayoutTests/SlowTests or not.  If SlowTests resolves
> the flakiness, we don't need to split tests.
> 
> If you'd like to land this soon, I recommend to add new tests (or copy a part
of
> reflection-tabular.html) to this CL, and add a comment about it to
> crbug.com/490939.

Sounds great, I added the test and can add a comment to crbug.com/490939 once it
lands.

[tkent, 2016-09-29 23:15:56]

lgtm

https://codereview.chromium.org/2376593003/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/table/table-cell-scope-getter.html
(right):

https://codereview.chromium.org/2376593003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/table/table-cell-scope-getter.html:8: td =
document.createElement('td');
indentation looks bad.

[tkent, 2016-09-30 06:51:02]

I imported reflection-tabular.html this morning, and it was not flaky.  This CL
may reply on it.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=wpt...

[rwlbuis, 2016-09-30 12:27:20]

On 2016/09/30 06:51:02, tkent wrote:
> I imported reflection-tabular.html this morning, and it was not flaky.  This
CL
> may reply on it.
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=wpt...

Ok I'll try to add it instead of fast/table/table-cell-scope-getter.html.

[rwlbuis, 2016-09-30 14:30:52]

The CQ bit was checked by rob.buis@samsung.com

[rwlbuis, 2016-09-30 14:30:52]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/2376593003/#ps80001
(title: "Patch for landing")

[commit-bot: I haz the power, 2016-09-30 14:31:10]

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, 2016-09-30 15:51:38]

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

[commit-bot: I haz the power, 2016-09-30 15:51:39]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[rwlbuis, 2016-09-30 15:57:56]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2016-09-30 15:58:07]

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, 2016-09-30 17:35:02]

Description was changed from

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string (for default).

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

to

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string (for default).

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

[commit-bot: I haz the power, 2016-09-30 17:35:04]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-30 17:38:17]

Description was changed from

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string (for default).

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope
==========

to

==========
Limit scope getter to predefined values

The scope attribute is limited to four predefined keywords [1], make
sure the idl binding will only return one of those keywords or
empty string (for default).

Behavior matches Safari and Firefox.

[1] https://html.spec.whatwg.org/#the-th-element:attr-th-scope

Committed: https://crrev.com/f3f53a039bcdda5bfda56f1f5ac5fd6f84b3e704
Cr-Commit-Position: refs/heads/master@{#422144}
==========

[commit-bot: I haz the power, 2016-09-30 17:38:18]

Patchset 5 (id:??) landed as
https://crrev.com/f3f53a039bcdda5bfda56f1f5ac5fd6f84b3e704
Cr-Commit-Position: refs/heads/master@{#422144}