adding/removing columns causes incorrect table width distribution |
||||
Issue descriptionChrome Version : 67.0.3396.99 (Official Build) (64-bit) URLs (if applicable) : https://jsfiddle.net/8odukjz0 Other browsers tested: Add OK or FAIL, along with the version, after other browsers where you have tested this issue: Safari: Firefox: OK Edge: What steps will reproduce the problem? (1) Head to the reproducer jsfiddle. This is based upon the WebKit/LayoutTests/tables/mozilla/dom/deleteCol1.html layout test (2) Hit the "Modify table" button What is the expected result? The two tables should look the same. What happens instead? Chrome incorrectly computes the first table so that it's much wider (200px extra) than it should be.
,
Aug 3
Able to reproduce the issue on Win-10 using chrome reported version #67.0.3396.99 and latest canary #70.0.3510.0. 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...!!
,
Aug 3
Also seems to behave incorrectly when adding colgroups too: https://jsfiddle.net/87z49L2s/ This is on Chrome 67.0.3396.99 as before, it also happens on the latest version I built (70.0.3507.0).
,
Aug 3
I think I've got a reasonable idea of what the solution might be (https://chromium-review.googlesource.com/c/chromium/src/+/1162224/3) but there's a layout test regression which needs fixing. I'm currently trying to figure out how best to do this.
,
Aug 6
+dgrogan Exciting Richard. dgrogan (cc:ed) would be the best reviewer for the change. Thank you!
,
Aug 13
On chrome 68 on linux, both https://jsfiddle.net/8odukjz0/3 and https://jsfiddle.net/87z49L2s/ look the same as the reference table (bottom, right?) after hitting Modify. IIUC, you are saying they are different. Am I missing something?
,
Aug 13
Not sure what might be going on, but make sure that your browser window is large enough: e.g. when I size the browser to half my screen size, it looks OK after hitting the modify button (layout-screenshot.png), but after resizing the window I see the issue (layout-screenshot-2.png)
,
Aug 13
Yes, that was it. My results window was < 600px, in which the table does not fit, causing other code to run.
,
Sep 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/29028a2dea465add1c1433d8260da89adb6709d2 commit 29028a2dea465add1c1433d8260da89adb6709d2 Author: Richard Townsend <Richard.Townsend@arm.com> Date: Tue Sep 18 18:17:43 2018 [css-tables] Force layout when colgroups are removed or added When colgroups are modified via Javascript, Blink uses a stale computed logical width, resulting in incorrect sizing of auto columns if layout was previously done. This patch forces a fresh layout of the affected cells, which corrects this behaviour. Bug: 870296 Change-Id: I4a12d5a8db689ba1457c6c9960ae8ea9c1dd40d9 Reviewed-on: https://chromium-review.googlesource.com/1162224 Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Cr-Commit-Position: refs/heads/master@{#592115} [add] https://crrev.com/29028a2dea465add1c1433d8260da89adb6709d2/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/width-distribution/computing-column-measure-2.html [modify] https://crrev.com/29028a2dea465add1c1433d8260da89adb6709d2/third_party/blink/renderer/core/layout/layout_table.cc [modify] https://crrev.com/29028a2dea465add1c1433d8260da89adb6709d2/third_party/blink/renderer/core/layout/layout_table.h
,
Sep 18
,
Sep 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66a74d2f7e1b369ce086c3c41ee089d72f3d0018 commit 66a74d2f7e1b369ce086c3c41ee089d72f3d0018 Author: Tommy Li <tommycli@chromium.org> Date: Tue Sep 18 22:59:12 2018 Revert "[css-tables] Force layout when colgroups are removed or added" This reverts commit 29028a2dea465add1c1433d8260da89adb6709d2. Reason for revert: Breaks Linux Msan tests. See https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20MSan%20Tests/builds/11648 Original change's description: > [css-tables] Force layout when colgroups are removed or added > > When colgroups are modified via Javascript, Blink uses a stale computed > logical width, resulting in incorrect sizing of auto columns if layout > was previously done. This patch forces a fresh layout of the affected > cells, which corrects this behaviour. > > Bug: 870296 > Change-Id: I4a12d5a8db689ba1457c6c9960ae8ea9c1dd40d9 > Reviewed-on: https://chromium-review.googlesource.com/1162224 > Commit-Queue: David Grogan <dgrogan@chromium.org> > Reviewed-by: Morten Stenshorne <mstensho@chromium.org> > Reviewed-by: Emil A Eklund <eae@chromium.org> > Reviewed-by: David Grogan <dgrogan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#592115} TBR=dgrogan@chromium.org,eae@chromium.org,richard.townsend@arm.com,mstensho@chromium.org Change-Id: If7507718dddd3b84ceffdae3df5e35b8402f27ea No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 870296 Reviewed-on: https://chromium-review.googlesource.com/1232480 Reviewed-by: Tommy Li <tommycli@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#592243} [delete] https://crrev.com/c262fce0a67456a0d3a5f9e0e2884adafff234e9/third_party/WebKit/LayoutTests/external/wpt/css/css-tables/width-distribution/computing-column-measure-2.html [modify] https://crrev.com/66a74d2f7e1b369ce086c3c41ee089d72f3d0018/third_party/blink/renderer/core/layout/layout_table.cc [modify] https://crrev.com/66a74d2f7e1b369ce086c3c41ee089d72f3d0018/third_party/blink/renderer/core/layout/layout_table.h
,
Sep 19
Looks like I forgot to initialise the new flag, reassuringly it's been flagged by MSAN. Fixing... |
||||
►
Sign in to add a comment |
||||
Comment 1 by swarnasree.mukkala@chromium.org
, Aug 2