Cells which span rows need to have layout invalidated on visibility change to adjacent rows |
|||
Issue descriptionChrome Version: (copy from chrome://version) OS: (e.g. Win7, OSX 10.9.5, etc...) What steps will reproduce the problem? (1)use flag --enable-experimental-web-platform-features (2)go to https://jsfiddle.net/dgrogan/sqah2j2L/ (3)click "change visibility" button twice What is the expected result? The browser shouldn't crash and the content in the spanning cell should toggle between the normal state and being collapsed. What happens instead? The browser crashes when you toggle from collapsed -> uncollapsed. FindPaintOffsetAndVisualRectNeedingUpdate::CheckVisualRect() fails. This only happens when content is clipped and collapsed. Please use labels and text to provide additional information. For graphics-related bugs, please copy/paste the contents of the about:gpu page at the end of this report.
,
Aug 9 2017
The bug is just as I thought: when changing 'visibility' CSS of one row of a table, the border box rect of adjacent rows which have rowspan > 1 may change, if those rows' rowspan included the row which was made to collapse via 'visibility: collapse'. To fix this, it may be you need to add code to find other rows for which to invalidate layout by iterating the rows and looking for rowspans, perhaps checking if those rowspans intersect the row just made collapsed.
,
Aug 10 2017
Joy, look at LayoutTableRow::StyleDidChange. Try something like
if ((old_style->Visibility() == EVisibility::kCollapse) !=
(Style()->Visibility() == EVisibility::kCollapse)) {
// iterate through all the rows in the section {
row->SetNeedsLayout();
}
}
If that fixes it then we can get smarter about only calling setNeedsLayout on the rows that need it by inspecting grid_ in LayoutTableSection.
,
Aug 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08f66e07f892c37693021a68088235bdc73836cc commit 08f66e07f892c37693021a68088235bdc73836cc Author: Joy Yu <joysyu@google.com> Date: Mon Aug 14 19:08:29 2017 Properly mark cell's children as needing layout. When a cell is spanning a collapsed row, it may get clipped. When the row is then uncollapsed, the cell's children need to be laid out again. Before, CellWidthChanged() determined whether the cell's children needed layout. Now, CellWidthChanged() has been more accurately renamed to CellChildrenNeedLayout(). Bug: 753515 Change-Id: Ic138066b8e2b4a96deba4e67c28348ae211ec7c0 Reviewed-on: https://chromium-review.googlesource.com/610894 Commit-Queue: Joy Yu <joysyu@google.com> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#494122} [add] https://crrev.com/08f66e07f892c37693021a68088235bdc73836cc/third_party/WebKit/LayoutTests/external/wpt/css/css-tables-3/visibility-collapse-rowspan-crash.html [modify] https://crrev.com/08f66e07f892c37693021a68088235bdc73836cc/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp [modify] https://crrev.com/08f66e07f892c37693021a68088235bdc73836cc/third_party/WebKit/Source/core/layout/LayoutTableCell.h [modify] https://crrev.com/08f66e07f892c37693021a68088235bdc73836cc/third_party/WebKit/Source/core/layout/LayoutTableRow.cpp
,
Aug 14 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by joysyu@google.com
, Aug 8 2017LayoutView 0x3b46b9604010 #document LayoutBlockFlow 0x3b46b9624010 HTML LayoutBlockFlow 0x3b46b9624130 BODY LayoutTable 0x3b46b96041a0 TABLE LayoutTableSection 0x3b46b9630010 TBODY LayoutTableRow 0x3b46b9638010 TR LayoutTableCell 0x3b46b963c010 TD LayoutText 0x3b46b9644010 #text "cell1" LayoutTableCell 0x3b46b963c148 TD LayoutBlockFlow 0x3b46b9624250 DIV id="overflowdiv" style="outline: 1px solid cyan" * LayoutText 0x3b46b96440d8 #text "\n Overflow Overflow Overflow" LayoutTableRow 0x3b46b9638120 TR id="rowToChange1" style="visibility: visible;" LayoutTableCell 0x3b46b963c280 TD LayoutText 0x3b46b96441a0 #text "hihi" LayoutBlockFlow (anonymous) 0x3b46b96245b0 LayoutButton (positioned) 0x3b46b9654010 BUTTON id="changeVisibility" style="position:absolute; top:80px" (focused) LayoutBlockFlow (anonymous) 0x3b46b9624370 LayoutText 0x3b46b9644268 #text "\n Change Visibility With Text\n" LayoutBlockFlow (positioned) 0x3b46b9624490 DIV style="position:absolute; top:115px" LayoutText 0x3b46b9644330 #text "\nClicks on text div: " LayoutInline 0x3b46b9660010 SPAN id="clickcount" LayoutText 0x3b46b96443f8 #text "0" LayoutText 0x3b46b96444c0 #text "\n" LayoutBR 0x3b46b9644588 BR LayoutBR 0x3b46b9644650 BR LayoutBR 0x3b46b9644718 BR LayoutBR 0x3b46b96447e0 BR LayoutTable 0x3b46b9604330 TABLE LayoutTableSection 0x3b46b9630180 TBODY LayoutTableRow 0x3b46b9638230 TR LayoutTableCell 0x3b46b963c3b8 TD LayoutText 0x3b46b96448a8 #text "cell1" LayoutTableCell 0x3b46b963c4f0 TD LayoutBlockFlow 0x3b46b96246d0 DIV id="overflowingnotextdiv" style="outline: 1px solid cyan; height: 200px; width: 50px;" LayoutTableRow 0x3b46b9638340 TR id="rowToChange" LayoutTableCell 0x3b46b963c628 TD LayoutText 0x3b46b9644970 #text "hihi" LayoutBlockFlow (anonymous) 0x3b46b96247f0 LayoutButton 0x3b46b9654190 BUTTON id="changeVisibilityNoText" LayoutBlockFlow (anonymous) 0x3b46b9624910 LayoutText 0x3b46b9644a38 #text "\n Change Visibility No Text\n" LayoutText 0x3b46b9644b00 #text "\n\n" [1:1:0808/140541.048996:686451584283:FATAL:FindPaintOffsetAndVisualRectNeedingUpdate.h(107)] Check failed: (old_visual_rect_.IsEmpty() && new_visual_rect.IsEmpty()) || object_.EnclosingLayer()->SubtreeIsInvisible() || old_visual_rect_ == new_visual_rect || (InflatedRect(old_visual_rect_).Contains(new_visual_rect) && InflatedRect(new_visual_rect).Contains(old_visual_rect_)). Visual rect changed without needing update object="LayoutText #text" old="57,26 241x10" new="57,26 241x23" #0 0x7f1659ee434d base::debug::StackTrace::StackTrace() #1 0x7f1659ee271c base::debug::StackTrace::StackTrace() #2 0x7f1659f72e3a logging::LogMessage::~LogMessage() #3 0x7f1651d6594c blink::FindVisualRectNeedingUpdateScopeBase::CheckVisualRect() #4 0x7f1652ae901b blink::FindObjectVisualRectNeedingUpdateScope::~FindObjectVisualRectNeedingUpdateScope() #5 0x7f1652ae848d blink::PaintInvalidator::UpdateVisualRectIfNeeded() #6 0x7f1652ae68f7 blink::PaintInvalidator::InvalidatePaint() #7 0x7f1652b4eabc blink::PrePaintTreeWalk::Walk() #8 0x7f1652b4ebba blink::PrePaintTreeWalk::Walk() #9 0x7f1652b4ebba blink::PrePaintTreeWalk::Walk() #10 0x7f1652b4ebba blink::PrePaintTreeWalk::Walk()