New issue
Advanced search Search tips

Element not repainted when changing its z-index from a minus number to 1.

Reported by weiw...@gmail.com, Sep 14 
UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.92 Safari/537.36

Steps to reproduce the problem:
1. Open the test page. The initial zIndex of element "virtual-cell" is -1. It is invisible.
2. Click the "Set z-index 1" button. The zIndex of element "virtual-cell" is changed to 1. However it doesn't change from invisible to visible.
3. Click the "Set z-index 2" button. The zIndex of element "virtual-cell" is changed to 2. Now it becomes visible.
4. Click the "Set z-index 1" button. The zIndex of element "virtual-cell" is changed back to 1. It keeps visible.
5. Click the "Set z-index -1" button. The zIndex of element "virtual-cell" is changed to -1. Now it becomes invisible.
6. Click the "Set z-index 1" button. The zIndex of element "virtual-cell" is changed to 1. Same as step2, it is not visible.
7. Click the "Append&Remove dummy dom". The zIndex of element "virtual-cell" is not changed. A dummy div will be appended, measured and removed. However now "virtual-cell" becomes visible.

What is the expected behavior?
Element should be repainted when changing its z-index from a minus number to 1.

What went wrong?
There is an element with a minus z-index (and absolute position). When changing its z-index to 1, it is not shown. However when changing its z-index to 2, it becomes visible. Moreover changing z-index back to 1, it keeps the visibility.

Inserting and removing an extra DOM will trigger the repainting. 

Did this work before? Yes 68.0.3440.106

Does this work in other browsers? Yes

Chrome version: 69.0.3497.92  Channel: stable
OS Version: 10.0
Flash Version: 

Live demo:
https://jsfiddle.net/mha13tck/
zindex-native-js.html
2.2 KB View Download

Comment 1 by pbomm...@chromium.org, Sep 14

Cc: pbomm...@chromium.org gov...@chromium.org
Components: Blink>Layout
Labels: ReleaseBlock-Stable M-69 Target-70 Target-71 RegressedIn-69 M-71 M-70 FoundIn-71 FoundIn-70 Target-69 FoundIn-69 OS-Android OS-Chrome OS-Linux OS-Mac
Owner: chrishtr@chromium.org
Status: Assigned (was: Unconfirmed)
This is a regression which started in M69 and see similar behavior on Chrome Beta(70.0.3538.16) , Dev(71.0.3551.3) and Canary(	71.0.3552.2)  and below is the bisect range :

You are probably looking for a change made after 570448 (known good), but no later than 570449 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/a76d0cdcee60e76492bd2f0aa68a2d391ceb98c7..88151691ad1b7388b29400a8ecf84ecc82fefcd1

Comment 2 by chrishtr@chromium.org, Sep 15

Labels: -Pri-2 Pri-1
This is a really bad bug. :(

Any change from any z-index other than zero to 1 will trigger the bug.

The good news is that the fix is trivial and safe. Working on that now.

Comment 4 by gov...@chromium.org, Sep 15

Cc: benmason@chromium.org cindyb@chromium.org
+benmason@ & cindyb@ (Chrome on Android and Chrome OS M69 TPMs)

Comment 5 by chrishtr@chromium.org, Sep 15

Labels: Merge-Request-69 Merge-Request-70
The fix is safe to merge directly to 69 because it simply stores an integer in
an integer variable rather than a bool, which lost precision. Otherwise the logic
is unchanged.

Comment 6 by chrishtr@chromium.org, Sep 15 

Also, the exact same variable was an int before I mistakenly made it a bool.
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 15

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by gov...@chromium.org, Sep 15

Labels: -Merge-Request-69 -Merge-Review-70 Merge-Approved-70 Merge-Approved-69
Approving merge for (https://chromium-review.googlesource.com/c/chromium/src/+/1227498) to M69 branch 3497 and M70 branch 3538 based on comments #2, #5 and per offline group chat. Pls merge after change lands in trunk.

Pls monitor this change on tonight's canary to make sure the change looks good in canary. Thank you.
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 15 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddf589fe9c1a2a11b962ce13d2789862858c087b

commit ddf589fe9c1a2a11b962ce13d2789862858c087b
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Sat Sep 15 04:29:22 2018

[PE] Z-index should be an int, not a bool.

Bug:  884185 

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic61f875cfb176266c1d103d7ae4741b34330afca
Reviewed-on: https://chromium-review.googlesource.com/1227498
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591574}
[modify] https://crrev.com/ddf589fe9c1a2a11b962ce13d2789862858c087b/third_party/blink/renderer/core/paint/paint_layer_stacking_node.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 15

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90ee68622c6a85f6743fee0a7e579c937c0c4dc3

commit 90ee68622c6a85f6743fee0a7e579c937c0c4dc3
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Sat Sep 15 04:44:37 2018

[PE] Z-index should be an int, not a bool.

Bug:  884185 

TBR=chrishtr@chromium.org

(cherry picked from commit ddf589fe9c1a2a11b962ce13d2789862858c087b)

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic61f875cfb176266c1d103d7ae4741b34330afca
Reviewed-on: https://chromium-review.googlesource.com/1227498
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#591574}
Reviewed-on: https://chromium-review.googlesource.com/1227603
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#947}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/90ee68622c6a85f6743fee0a7e579c937c0c4dc3/third_party/blink/renderer/core/paint/paint_layer_stacking_node.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 15

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77b4ccb09fe28353156860fd5c242845290e6538

commit 77b4ccb09fe28353156860fd5c242845290e6538
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Sat Sep 15 04:47:40 2018

[PE] Z-index should be an int, not a bool.

Bug:  884185 

TBR=chrishtr@chromium.org

(cherry picked from commit ddf589fe9c1a2a11b962ce13d2789862858c087b)

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic61f875cfb176266c1d103d7ae4741b34330afca
Reviewed-on: https://chromium-review.googlesource.com/1227498
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#591574}
Reviewed-on: https://chromium-review.googlesource.com/1227507
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#433}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/77b4ccb09fe28353156860fd5c242845290e6538/third_party/blink/renderer/core/paint/paint_layer_stacking_node.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 15

Labels: merge-merged-3553
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8bf4a1d4f91bb9d1b043ead3d5dbb485d61a63ce

commit 8bf4a1d4f91bb9d1b043ead3d5dbb485d61a63ce
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Sat Sep 15 04:55:10 2018

[PE] Z-index should be an int, not a bool.

Bug:  884185 

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic61f875cfb176266c1d103d7ae4741b34330afca
Reviewed-on: https://chromium-review.googlesource.com/1227498
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#591574}(cherry picked from commit ddf589fe9c1a2a11b962ce13d2789862858c087b)
Reviewed-on: https://chromium-review.googlesource.com/1227508
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/3553@{#3}
Cr-Branched-From: d97d105fb1575487cfce16135bbc1faf9642e14b-refs/heads/master@{#591521}
[modify] https://crrev.com/8bf4a1d4f91bb9d1b043ead3d5dbb485d61a63ce/third_party/blink/renderer/core/paint/paint_layer_stacking_node.cc

Comment 13 by gov...@chromium.org, Sep 15 

Thank you  chrishtr@ for quick fix,land and merge to M69 and M70. 

Triggered new M69 RC for Android and Desktop.
Also merged the change to canary branch 3553 at #12 and triggered new Android and Desktop canary from same branch.

Comment 14 by chrishtr@chromium.org, Sep 15

Status: Fixed (was: Assigned)

Comment 15 by gov...@chromium.org, Sep 15 

Pls verify this change on M69 stable #69.0.3497.100 (currently building) and canary version Canary #71.0.3553.2 or higher. Thank you.

Comment 16 by chrishtr@chromium.org, Sep 15 

Canary 71.0.3553.2 looks good.

Stable not available yet on my Mac.

Comment 17 by chrishtr@chromium.org, Sep 16 

Verified also on 69.0.3497.100, working correctly.

Comment 18 by nyerramilli@chromium.org, Sep 17

Cc: nyerramilli@chromium.org
Labels: TE-Verified-M69 TE-Verified-69.0.3497.100
Just to update, verified the fix on 69.0.3497.100 on Win 10, Mac OS X 10.13.6, Debian Rodete - fix working as intended.

Attached screencast for reference, adding TE-Verified labels.

note: Able to reproduce the issue on 69.0.3497.92

Comment 19 by phanindra.mandapaka@chromium.org, Sep 17

Cc: phanindra.mandapaka@chromium.org
 Issue 884524  has been merged into this issue.

Comment 20 by weiw...@gmail.com, Sep 17 

Thank you for your quick response!

I have confirm that the regression is already fixed on 71.0.3555.0 .

https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Win_x64/591619/

Comment 21 by nyerramilli@chromium.org, Sep 17 

missed attachment in C#18, attaching now..
884185.mov
15.9 MB Download

Comment 22 by vamshi.kommuri@chromium.org, Sep 17

Labels: TE-Verified-M71 TE-Verified-71.0.3554.0
Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #71.0.3554.0 as per the comment #0.
Attaching screen cast for reference.
Observed that element repainted when changing its z-index from a minus number to 1.
Hence, the fix is working as expected. 
Adding the verified labels.
Note: Able to reproduce the issue on chrome version with out fix.

Thanks...!!
884185.mp4
816 KB View Download

Comment 23 by gov...@chromium.org, Sep 17 

Good news: 69.0.3497.100  is now available with the fix on Chrome Stable. You can force an update by visiting chrome://chrome. Thank you.
Project Member

Comment 24 by bugdroid1@chromium.org, Sep 17 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c19a1113cf2dcfd2599c200a65bba65ed8a5e0f

commit 0c19a1113cf2dcfd2599c200a65bba65ed8a5e0f
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Sep 17 19:39:39 2018

[PE] Add a test for z-index change from negative to positive.

This complements r591574, which landed without a test to speed
up cherrypicks.

Bug:  884185 

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: If658eac77dcab7d21b19c9e0472bedd7acc26985
Reviewed-on: https://chromium-review.googlesource.com/1227509
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591783}
[modify] https://crrev.com/0c19a1113cf2dcfd2599c200a65bba65ed8a5e0f/third_party/blink/renderer/core/paint/paint_layer_test.cc

Comment 25 by vamshi.kommuri@chromium.org, Sep 19

Labels: TE-Verified-M70 TE-Verified-70.0.3538.22
Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #70.0.3538.22 as per the comment #0.
Attaching screen cast for reference.
Observed that element repainted when changing its z-index from a minus number to 1.
Hence, the fix is working as expected. 
Adding the verified labels.
Note: Able to reproduce the issue on chrome version with out fix.

Thanks...!!
884185 CL.mp4
2.1 MB View Download

Comment 26 by gov...@chromium.org, Oct 1 

Requesting a postmortem for this, pls see go/chrome-postmortems for more details.
►

Sign in to add a comment