New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 596586 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Should updateLogicalWidthAndColumnWidth check for the content width changed?

Project Member Reported by cbiesin...@chromium.org, Mar 21 2016

Issue description

See this code in LayoutBlockFlow::layoutBlockFlow:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp&q=layoutBlockFlow&sq=package:chromium&l=352

We force-relayout the children if our logicalWidth() changed (and a couple other cases, see https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/layout/LayoutBlock.cpp&sq=package:chromium&l=940&rcl=1458545209)

As far as I can tell, this is for the case of our children being sized to fill-available, the default. But why is the logicalWidth() the one that matters for that? Should it be contentWidth()? Or am I misinterpreting what this is used for?
 

Comment 1 by e...@chromium.org, Mar 23 2016

Owner: le...@chromium.org
Status: Assigned (was: Untriaged)
Levi, what's your take on this?

Comment 2 by le...@chromium.org, Mar 23 2016

Cc: msten...@opera.com
Owner: cbiesin...@chromium.org
No, you're right. It appears that multicol handles this properly: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp&sq=package:chromium&l=313&rcl=1458732535
Levi, I'm confused, doesn't that actually have the same issue?

Comment 4 by le...@chromium.org, Mar 23 2016

Yeah, you're right again. We're still not looking at the column *content* width.

Comment 5 by msten...@opera.com, Mar 29 2016

Content box width is by far the most interesting value for children to look at. Padding box width is also interesting for absolutely positioned descendants. Border box width (logicalWidth()) shouldn't really be of any interest at all to any descendant. :) But very interesting to the layout engine, obviously, since that's what the engine typically stores and works with (LayoutBox::m_frameRect).

Then there's widthAvailableToChildrenHasChanged() in updateLogicalWidthAndColumnWidth(), which is used to detect border and padding changes in styleDidChange().

It's hard to properly detect a content box change during layout, since all we have is old and new border box, but only the *current* style. The style may have changed since previous layout, and we have no way to know what it used to be. But we do have (width|height)AvailableToChildrenHasChanged, which is used to make sure that children are laid out, even if the border box hasn't changed.

There is at least one other way (apart from style changes) for the content box to change without changing the border box, though: percentage padding (e.g. "width:500px; padding:10%; box-sizing:border-box;" and then resize the containing block of that element to change used padding). We have needsPreferredWidthsRecalculation() (which is a bad name for its use) to help us there.

There's room for cleanup and improvements in this area, but do we have bugs because of how things are currently being done?
I suppose we probably don't have bugs, with this code ensuring that changed padding or border sets widthAvailableToChildrenHasChanged():
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/layout/LayoutBlock.cpp&sq=package:chromium&l=358&rcl=1460561050

and scrollbarChanged() doing the same for scrollbars.

I suppose this is all OK, it just took me a while to understand it.

Owner: ----
Status: Available (was: Assigned)

Comment 8 by e...@chromium.org, Feb 18 2017

Is this still relevant?

Comment 9 by msten...@opera.com, Feb 20 2017

I don't think it's necessary to keep this issue open, if all it could potentially be about is the cleanup I'm talking about in #5. Unless we have actual bugs that we want to address, of course.

Christian?
Cc: -msten...@opera.com mstensho@chromium.org
Status: WontFix (was: Available)
I don't see any point in keeping this one open anymore.

Sign in to add a comment