Should updateLogicalWidthAndColumnWidth check for the content width changed? |
||||
Issue descriptionSee 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?
,
Mar 23 2016
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
,
Mar 23 2016
Levi, I'm confused, doesn't that actually have the same issue?
,
Mar 23 2016
Yeah, you're right again. We're still not looking at the column *content* width.
,
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?
,
Apr 13 2016
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.
,
May 12 2016
,
Feb 18 2017
Is this still relevant?
,
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?
,
Jan 31 2018
I don't see any point in keeping this one open anymore. |
||||
►
Sign in to add a comment |
||||
Comment 1 by e...@chromium.org
, Mar 23 2016Status: Assigned (was: Untriaged)