Text control elements should contain all (shadow DOM) children.

Text control elements should contain all (shadow DOM) children.

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want authors
to be able to escape the containing INPUT element by styling the cancel button as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain of
all its descendants. This should be a good idea in general (and at least harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Since this change makes canContainFixedPositionObjects() in LayoutObject and
ComputedStyle even more different than they used to be, we need to change some
code from using the one in ComputedStyle to the one in LayoutObject.

Two existing tests in fast/forms/ had to be updated, since they were adding together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT element
in a way that used to work by accident. Use getBoundingClientRect() instead, since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2733593002
Cr-Commit-Position: refs/heads/master@{#455644}
Committed: https://chromium.googlesource.com/chromium/src/+/8f4b7a0506f076b26b76dd6aaf624860a6d3fda1

[mstensho (USE GERRIT), 2017-03-03 15:53:00]

Description was changed from

==========
User agent shadow hosts should contain out-of-flow descendants.

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector.

BUG=685527
==========

to

==========
User agent shadow hosts should contain out-of-flow descendants.

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector.

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mstensho (USE GERRIT), 2017-03-03 15:53:05]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-03 15:53:18]

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-03-03 16:24:44]

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

[commit-bot: I haz the power, 2017-03-03 16:24:45]

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

[mstensho (USE GERRIT), 2017-03-03 22:20:32]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-03 22:20:59]

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-03-03 22:57:56]

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

[commit-bot: I haz the power, 2017-03-03 22:57:56]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[mstensho (USE GERRIT), 2017-03-07 15:18:48]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 15:19:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mstensho (USE GERRIT), 2017-03-07 15:24:14]

Description was changed from

==========
User agent shadow hosts should contain out-of-flow descendants.

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector.

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
INPUT elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT elements to be in the containing block chain of all its descendants.
This
should be a good idea in general (and at least harmless), and there's also C++
code [1]
that essentially assumes that it is so.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2017-03-07 16:33:29]

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

[commit-bot: I haz the power, 2017-03-07 16:33:30]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mstensho (USE GERRIT), 2017-03-08 12:53:15]

Description was changed from

==========
INPUT elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT elements to be in the containing block chain of all its descendants.
This
should be a good idea in general (and at least harmless), and there's also C++
code [1]
that essentially assumes that it is so.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
INPUT elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT elements to be in the containing block chain of all its descendants.
This
should be a good idea in general (and at least harmless), and there's also C++
code [1]
that essentially assumes that it is so.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test wants absolute coordinates in the end anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mstensho (USE GERRIT), 2017-03-08 12:54:03]

Description was changed from

==========
INPUT elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT elements to be in the containing block chain of all its descendants.
This
should be a good idea in general (and at least harmless), and there's also C++
code [1]
that essentially assumes that it is so.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test wants absolute coordinates in the end anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Text control elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT elements to be in the containing block chain of all its descendants.
This
should be a good idea in general (and at least harmless), and there's also C++
code [1]
that essentially assumes that it is so.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test wants absolute coordinates in the end anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mstensho (USE GERRIT), 2017-03-08 12:55:36]

Description was changed from

==========
Text control elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT elements to be in the containing block chain of all its descendants.
This
should be a good idea in general (and at least harmless), and there's also C++
code [1]
that essentially assumes that it is so.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test wants absolute coordinates in the end anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Text control elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain
of
all its descendants. This should be a good idea in general (and at least
harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mstensho (USE GERRIT), 2017-03-08 13:15:43]

Description was changed from

==========
Text control elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain
of
all its descendants. This should be a good idea in general (and at least
harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Text control elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain
of
all its descendants. This should be a good idea in general (and at least
harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Since this change makes canContainFixedPositionObjects() in LayoutObject and
ComputedStyle even more different than they used to be, we need to change some
code from using the one in ComputedStyle to the one in LayoutObject.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mstensho (USE GERRIT), 2017-03-08 13:16:23]

Description was changed from

==========
Text control elements should contain all (shadow DOM) children

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain
of
all its descendants. This should be a good idea in general (and at least
harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Since this change makes canContainFixedPositionObjects() in LayoutObject and
ComputedStyle even more different than they used to be, we need to change some
code from using the one in ComputedStyle to the one in LayoutObject.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Text control elements should contain all (shadow DOM) children.

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain
of
all its descendants. This should be a good idea in general (and at least
harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Since this change makes canContainFixedPositionObjects() in LayoutObject and
ComputedStyle even more different than they used to be, we need to change some
code from using the one in ComputedStyle to the one in LayoutObject.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[mstensho (USE GERRIT), 2017-03-08 13:16:41]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 13:16: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-03-08 15:12:42]

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

[commit-bot: I haz the power, 2017-03-08 15:12:43]

Dry run: This issue passed the CQ dry run.

[mstensho (USE GERRIT), 2017-03-08 17:55:12]

mstensho@opera.com changed reviewers:
+ eae@chromium.org, wangxianzhu@chromium.org

[Xianzhu, 2017-03-08 18:20:06]

I would ask some questions:
1. Should we apply such rule to all shadow DOM roots?
2. Can we just simply ignore such children, like what we do in
LayoutMedia::isChildAllowed()?

[mstensho (USE GERRIT), 2017-03-08 19:11:14]

On 2017/03/08 18:20:06, Xianzhu wrote:
> I would ask some questions:
> 1. Should we apply such rule to all shadow DOM roots?

Doing that on author-defined shadow DOM roots would violate things, wouldn't it
(unless some spec somewhere actually says that this is how to behave)?
But maybe we could do it for all user agent shadow DOM roots, at least. That was
what I tried first, BTW; see patch set 1. I later ended up with the current
proposal, because lazy me found no easy way of checking for this condition.

> 2. Can we just simply ignore such children, like what we do in
> LayoutMedia::isChildAllowed()?

In this case it's about an abspos child inside a flexbox inside the INPUT
element, so that wouldn't work then, would it?

[Xianzhu, 2017-03-08 19:20:55]

lgtm. Please also wait for eae@'s feedback.

On 2017/03/08 19:11:14, mstensho wrote:
> On 2017/03/08 18:20:06, Xianzhu wrote:
> > I would ask some questions:
> > 1. Should we apply such rule to all shadow DOM roots?
> 
> Doing that on author-defined shadow DOM roots would violate things, wouldn't
it
> (unless some spec somewhere actually says that this is how to behave)?
> But maybe we could do it for all user agent shadow DOM roots, at least. That
was
> what I tried first, BTW; see patch set 1. I later ended up with the current
> proposal, because lazy me found no easy way of checking for this condition.
> 
> > 2. Can we just simply ignore such children, like what we do in
> > LayoutMedia::isChildAllowed()?
> 
> In this case it's about an abspos child inside a flexbox inside the INPUT
> element, so that wouldn't work then, would it?

Right. It seems that the LayoutMedia::isChildAllowed() can't completely avoid
such situations either.

[eae, 2017-03-09 01:30:10]

Good catch, I had assumed we already enforced this.

Thank you.

LGTM

[Xianzhu, 2017-03-09 02:22:59]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-03-09 02:23:32]

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-03-09 02:28:55]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1489026179060980,
"parent_rev": "c59fcd06cbe444483d1a53c30ae4dc44544957ff", "commit_rev":
"8f4b7a0506f076b26b76dd6aaf624860a6d3fda1"}

[commit-bot: I haz the power, 2017-03-09 02:29:40]

Description was changed from

==========
Text control elements should contain all (shadow DOM) children.

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain
of
all its descendants. This should be a good idea in general (and at least
harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Since this change makes canContainFixedPositionObjects() in LayoutObject and
ComputedStyle even more different than they used to be, we need to change some
code from using the one in ComputedStyle to the one in LayoutObject.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Text control elements should contain all (shadow DOM) children.

For instance, INPUT type="search" elements allow styling of the cancel button,
via a ::-webkit-search-cancel-button pseudo element selector. We don't want
authors
to be able to escape the containing INPUT element by styling the cancel button
as
position:absolute, etc.

Force INPUT and other text control elements to be in the containing block chain
of
all its descendants. This should be a good idea in general (and at least
harmless),
and there's also C++ code [1] that essentially assumes that it is so.

Since this change makes canContainFixedPositionObjects() in LayoutObject and
ComputedStyle even more different than they used to be, we need to change some
code from using the one in ComputedStyle to the one in LayoutObject.

Two existing tests in fast/forms/ had to be updated, since they were adding
together
offsetLeft of an INPUT element and offsetLeft of something inside the INPUT
element
in a way that used to work by accident. Use getBoundingClientRect() instead,
since
the test ultimately wants absolute coordinates anyway.

[1] See ThemePainterDefault::paintSearchFieldCancelButton()

BUG=685527
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2733593002
Cr-Commit-Position: refs/heads/master@{#455644}
Committed:
https://chromium.googlesource.com/chromium/src/+/8f4b7a0506f076b26b76dd6aaf62...
==========

[commit-bot: I haz the power, 2017-03-09 02:29:41]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/8f4b7a0506f076b26b76dd6aaf62...