Forces stacking context for computed value of transform-style:preserve-3d

Forces stacking context for computed value of transform-style:preserve-3d

This CL reverts the behavior change due to a side effect of r401197.
Prior to r401197, the used value of z-index is adjusted prior to resolving
the used value of transform-style. i.e. adjusted using the computed value.

According to the current spec, overflow other than 'visible' should force
the used value of transform-style:flat even if the computed value was
'preserve-3d'. The spec is unclear about how to resolve z-index in this case.

BUG=663650
Committed: https://crrev.com/413965df09a5ffbcdfa37f82c273af2fdf659654
Cr-Commit-Position: refs/heads/master@{#435439}

[trchen, 2016-11-29 04:08:54]

trchen@chromium.org changed reviewers:
+ chrishtr@chromium.org

[trchen, 2016-11-29 04:08:55]

This is an alternative fix for crbug.com/663650.

I personally think the existing behavior is buggy, but I agree reverting to
existing behavior is usually okay. :/

[trchen, 2016-11-29 04:08:59]

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

[commit-bot: I haz the power, 2016-11-29 04:09:16]

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

[chrishtr, 2016-11-29 04:54:02]

To further clarify: which browsers will we match after this CL?

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/transforms/computed-transform-style-forces-stacking-context.html
(right):

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/transforms/computed-transform-style-forces-stacking-context.html:2:
<div style="position:relative; transform-style:preserve-3d; overflow:hidden;
width:100px; height:100px;">
Please test this in ComputedStyleTest.cpp instead.

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1210: // Used value of
transform-style other than 'flat' already forces stacking
Suggest rephrasing for clarity:

"Force a stacking context for transform-style: preserve-3d.
This happens even if preserves-3d is ignored due to 
a 'grouping property' being present which requires flattening.
See ComputedStyle::usedTransformStyle3D() and
ComputedStyle::hasGroupingProperty().

This is legacy behavior that is left ambiguous in the official specs.
See crbug.com/663650 for more details."

[commit-bot: I haz the power, 2016-11-29 05:53:19]

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

[commit-bot: I haz the power, 2016-11-29 05:53:19]

Dry run: This issue passed the CQ dry run.

[trchen, 2016-11-30 01:26:23]

On 2016/11/29 04:54:02, chrishtr wrote:
> To further clarify: which browsers will we match after this CL?

Chrome M52 or anything prior to r401197.

As for other vendors, the overall behavior is too different to make meaningful
comparison. For example, not every vendor forces flattening on overflow:hidden.

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/transforms/computed-transform-style-forces-stacking-context.html
(right):

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/transforms/computed-transform-style-forces-stacking-context.html:2:
<div style="position:relative; transform-style:preserve-3d; overflow:hidden;
width:100px; height:100px;">
On 2016/11/29 04:54:02, chrishtr wrote:
> Please test this in ComputedStyleTest.cpp instead.

Done.

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1210: // Used value of
transform-style other than 'flat' already forces stacking
On 2016/11/29 04:54:02, chrishtr wrote:
> Suggest rephrasing for clarity:
> 
> "Force a stacking context for transform-style: preserve-3d.
> This happens even if preserves-3d is ignored due to 
> a 'grouping property' being present which requires flattening.
> See ComputedStyle::usedTransformStyle3D() and
> ComputedStyle::hasGroupingProperty().
> 
> This is legacy behavior that is left ambiguous in the official specs.
> See crbug.com/663650 for more details."

Done.

[chrishtr, 2016-11-30 01:30:17]

On 2016/11/30 at 01:26:23, trchen wrote:
> On 2016/11/29 04:54:02, chrishtr wrote:
> > To further clarify: which browsers will we match after this CL?
> 
> Chrome M52 or anything prior to r401197.
> 
> As for other vendors, the overall behavior is too different to make meaningful
comparison. For example, not every vendor forces flattening on overflow:hidden.

Do any of them force a stacking context in cases where any grouping properties
force flattening?

> 
>
https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/LayoutTe...
> File
third_party/WebKit/LayoutTests/transforms/computed-transform-style-forces-stacking-context.html
(right):
> 
>
https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/LayoutTe...
>
third_party/WebKit/LayoutTests/transforms/computed-transform-style-forces-stacking-context.html:2:
<div style="position:relative; transform-style:preserve-3d; overflow:hidden;
width:100px; height:100px;">
> On 2016/11/29 04:54:02, chrishtr wrote:
> > Please test this in ComputedStyleTest.cpp instead.
> 
> Done.
> 
>
https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):
> 
>
https://codereview.chromium.org/2532223003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/style/ComputedStyle.cpp:1210: // Used value of
transform-style other than 'flat' already forces stacking
> On 2016/11/29 04:54:02, chrishtr wrote:
> > Suggest rephrasing for clarity:
> > 
> > "Force a stacking context for transform-style: preserve-3d.
> > This happens even if preserves-3d is ignored due to 
> > a 'grouping property' being present which requires flattening.
> > See ComputedStyle::usedTransformStyle3D() and
> > ComputedStyle::hasGroupingProperty().
> > 
> > This is legacy behavior that is left ambiguous in the official specs.
> > See crbug.com/663650 for more details."
> 
> Done.

[trchen, 2016-11-30 01:39:43]

On 2016/11/30 01:30:17, chrishtr wrote:
> On 2016/11/30 at 01:26:23, trchen wrote:
> > On 2016/11/29 04:54:02, chrishtr wrote:
> > > To further clarify: which browsers will we match after this CL?
> > 
> > Chrome M52 or anything prior to r401197.
> > 
> > As for other vendors, the overall behavior is too different to make
meaningful
> comparison. For example, not every vendor forces flattening on
overflow:hidden.
> 
> Do any of them force a stacking context in cases where any grouping properties
> force flattening?

Overflow is special because it should not affect stacking context status. Every
other grouping properties force stacking context themselves anyway. (This is one
reason why I personally feel overflow shouldn't be grouping property nor it
should force flattening.)

[chrishtr, 2016-11-30 01:41:05]

On 2016/11/30 at 01:39:43, trchen wrote:
> On 2016/11/30 01:30:17, chrishtr wrote:
> > On 2016/11/30 at 01:26:23, trchen wrote:
> > > On 2016/11/29 04:54:02, chrishtr wrote:
> > > > To further clarify: which browsers will we match after this CL?
> > > 
> > > Chrome M52 or anything prior to r401197.
> > > 
> > > As for other vendors, the overall behavior is too different to make
meaningful
> > comparison. For example, not every vendor forces flattening on
overflow:hidden.
> > 
> > Do any of them force a stacking context in cases where any grouping
properties
> > force flattening?
> 
> Overflow is special because it should not affect stacking context status.
Every other grouping properties force stacking context themselves anyway. (This
is one reason why I personally feel overflow shouldn't be grouping property nor
it should force flattening.)

So there is no other browser that has the exact behavior of Chrome on the
example in your first patchset's layout test?

[trchen, 2016-11-30 01:58:20]

On 2016/11/30 01:41:05, chrishtr wrote:
> On 2016/11/30 at 01:39:43, trchen wrote:
> > On 2016/11/30 01:30:17, chrishtr wrote:
> > > On 2016/11/30 at 01:26:23, trchen wrote:
> > > > On 2016/11/29 04:54:02, chrishtr wrote:
> > > > > To further clarify: which browsers will we match after this CL?
> > > > 
> > > > Chrome M52 or anything prior to r401197.
> > > > 
> > > > As for other vendors, the overall behavior is too different to make
> meaningful
> > > comparison. For example, not every vendor forces flattening on
> overflow:hidden.
> > > 
> > > Do any of them force a stacking context in cases where any grouping
> properties
> > > force flattening?
> > 
> > Overflow is special because it should not affect stacking context status.
> Every other grouping properties force stacking context themselves anyway.
(This
> is one reason why I personally feel overflow shouldn't be grouping property
nor
> it should force flattening.)
> 
> So there is no other browser that has the exact behavior of Chrome on the
> example in your first patchset's layout test?

Honestly I don't know. The inspector in Edge & Firefox show "overflow:hidden;
transform-style:preserve-3d; z-index:auto;" but somehow the rendering still
results in a green box. It may be that their inspector only shows computed style
not used style, or overflow clip just don't force flattening.

[chrishtr, 2016-11-30 03:31:20]

lgtm

[chrishtr, 2016-11-30 03:31:27]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2016-11-30 03:32:36]

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-11-30 05:34:25]

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

[commit-bot: I haz the power, 2016-11-30 05:34:26]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[trchen, 2016-11-30 19:43:58]

The CQ bit was checked by trchen@chromium.org

[commit-bot: I haz the power, 2016-11-30 19:44:24]

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-11-30 22:11:07]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480535038807900,
"parent_rev": "575aed102db7abbfcc9dfccf32271ad029d8a80a", "commit_rev":
"a317a0112af5cb424ea5acffb2f3862240e95ffe"}

[commit-bot: I haz the power, 2016-11-30 22:11:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-30 22:15:18]

Description was changed from

==========
Forces stacking context for computed value of transform-style:preserve-3d

This CL reverts the behavior change due to a side effect of r401197.
Prior to r401197, the used value of z-index is adjusted prior to resolving
the used value of transform-style. i.e. adjusted using the computed value.

According to the current spec, overflow other than 'visible' should force
the used value of transform-style:flat even if the computed value was
'preserve-3d'. The spec is unclear about how to resolve z-index in this case.

BUG=663650
==========

to

==========
Forces stacking context for computed value of transform-style:preserve-3d

This CL reverts the behavior change due to a side effect of r401197.
Prior to r401197, the used value of z-index is adjusted prior to resolving
the used value of transform-style. i.e. adjusted using the computed value.

According to the current spec, overflow other than 'visible' should force
the used value of transform-style:flat even if the computed value was
'preserve-3d'. The spec is unclear about how to resolve z-index in this case.

BUG=663650

Committed: https://crrev.com/413965df09a5ffbcdfa37f82c273af2fdf659654
Cr-Commit-Position: refs/heads/master@{#435439}
==========

[commit-bot: I haz the power, 2016-11-30 22:15:19]

Patchset 2 (id:??) landed as
https://crrev.com/413965df09a5ffbcdfa37f82c273af2fdf659654
Cr-Commit-Position: refs/heads/master@{#435439}