New issue
Advanced search Search tips

Issue 837258 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Replace a col in a table do not force ui refresh

Reported by sylvain....@gmail.com, Apr 26 2018

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.117 Safari/537.36

Steps to reproduce the problem:
1. In the joined test case click the button "1" : the script remove the first <col> and insert a new one with an other class attribute.

What is the expected behavior?
The background of the first column should be green.

What went wrong?
No change.
In order to view the green background, you need to click the button "2" : a <tr> is removed and inserted that force a ui redraw.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 66.0.3359.117  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: 

Bug also in Windows Chrome Canary 68.0.3409.0 and in Linux Chrome 66.0.3359.117

No problem in Firefox 59.0.2 (windows and linux).
 
bugRefreshCol.html
1.2 KB View Download

Comment 1 by woxxom@gmail.com, Apr 26 2018

Bisected to 62ad1c5bc2e538f2a57c2e16e06327414a6fbf7a = https://crrev.com/2786463004 by wangxianzhu@chromium.org
"Paint backgrounds of a table section/row in one display item"
Landed in 59.0.3063.0
Labels: Needs-Triage-M66
Labels: Triaged-ET M-68 FoundIn-68 Target-68 OS-Linux OS-Mac
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on Mac 10.13.3, Win-10 and Ubuntu 14.04 using chrome reported version #66.0.3359.117 and latest canary #68.0.3409.2.
This is a non-regression issue as it is observed from M60 old builds. 

Hence, marking it as untriaged to get more inputs from dev team.

Thanks...!!
Components: -Blink>DOM Blink>CSS
Seems like a style invalidation problem.

Comment 5 by e...@chromium.org, May 2 2018

Cc: dgro...@chromium.org futhark@chromium.org
Status: Available (was: Untriaged)
Cc: wangxianzhu@chromium.org
Components: -Blink>CSS Blink>Layout>Table
Labels: -Type-Bug Type-Bug-Regression
Not sure if this is a paint or layout issue: I wonder what happens if the new column is assigned a width different than the old one.

Before the refactoring in https://crrev.com/2786463004 this might have just worked by accident. In any case, it's a regression.

Thanks for the bisect woxxom.
Components: -Blink>Layout>Table Blink>Paint>Invalidation
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Available)
It should be a paint invalidation issue. We miss a invalidation of the row display item client.
>I wonder what happens if the new column is assigned a width different than the old one.

Test done: no problem if width is changed, the new width is instantly visible. And the relayout force a repaint: the column is instantly green.
Project Member

Comment 9 by bugdroid1@chromium.org, May 5 2018

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

commit 045386cdc1006d3eb9b580491433bc6177832519
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Sat May 05 00:15:23 2018

[PE] Ensure invalidation of table section when any table col is replaced

Table section paints table col's background on its display item, so when
table col's background changes, we invalidate all table sections.
See https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/table_paint_invalidator.cc?rcl=597a899086d4baebb543eb55b7bd283a3b2068a1&l=43

Now let LayoutObject::BackgroundChangedSinceLastPaintInvalidation() be
initially true, so that a new col will be treated as background changed.

Bug:  837258 
Change-Id: Ic8efcbeb45ee879106f045b262fc4ee8e27cf891
Reviewed-on: https://chromium-review.googlesource.com/1043350
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556266}
[add] https://crrev.com/045386cdc1006d3eb9b580491433bc6177832519/third_party/WebKit/LayoutTests/paint/invalidation/table/replace-col-expected.html
[add] https://crrev.com/045386cdc1006d3eb9b580491433bc6177832519/third_party/WebKit/LayoutTests/paint/invalidation/table/replace-col-expected.txt
[add] https://crrev.com/045386cdc1006d3eb9b580491433bc6177832519/third_party/WebKit/LayoutTests/paint/invalidation/table/replace-col.html
[add] https://crrev.com/045386cdc1006d3eb9b580491433bc6177832519/third_party/WebKit/LayoutTests/virtual/disable-spv175/paint/invalidation/table/replace-col-expected.txt
[modify] https://crrev.com/045386cdc1006d3eb9b580491433bc6177832519/third_party/blink/renderer/core/layout/layout_object.h

Status: Fixed (was: Assigned)

Sign in to add a comment