If the block is inline-block and the preferred width of child needs to be recalculated, ...
3 years, 8 months ago
(2017-04-19 00:51:30 UTC)
#7
If the block is inline-block and the preferred width of child needs to be
recalculated, the preferred width of inline-block also should be recalculated.
So, I changed the code as you commented.
Thanks.
Sunghoon Kim
Description was changed from ========== Make inline-block container's width update Make inline-block container's preferred width ...
3 years, 8 months ago
(2017-04-19 04:12:00 UTC)
#8
Description was changed from
==========
Make inline-block container's width update
Make inline-block container's preferred width update
if its child's preferred width needs to be recalculated.
BUG=707151, 692874
==========
to
==========
Make inline-block container's width update
Make inline-block container's preferred width update
if its child's preferred width needs to be recalculated.
BUG=707151, 514663
==========
mstensho (USE GERRIT)
https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html File third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html (right): https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html#newcode13 third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html:13: width: auto; width and height are initially auto, so ...
3 years, 8 months ago
(2017-04-19 10:58:47 UTC)
#9
https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html
(right):
https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html:13:
width: auto;
width and height are initially auto, so no need to be explicit about it.
https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html:31:
<script src="../../resources/check-layout.js"></script>
Please consider using testharness when writing check-layout tests. I.e. include
the following scripts instead:
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../../resources/check-layout-th.js"></script>
That will eliminate the need for the -expected.txt counterpart.
https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/inline-block/inline-block-update-width.html:34:
function runTest() {
Seems that you can just do all these things right away, rather than hooking them
on an onload event. So no need for the wrapper function.
https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):
https://codereview.chromium.org/2828453002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:610:
IsInlineBlockOrInlineTable() ? kMarkContainerChain : kMarkOnlyThis);
Marking the container chain during layout seems wrong. This situation needs to
be detected at the beginning of layout of the shrink-to-fit container, not when
laying out its children.
Furthermore:
1. The inline-block may not be the immediate parent of the child that
NeedsPreferredWidthsRecalculation().
And:
2. inline-block isn't the only characteristic we should check for. Floats with
auto width will have similar issues. Table cells too, probably. And possibly
more.
Here's a test that demonstrates both #1 and #2. The suggested CL doesn't fix it:
<!DOCTYPE html>
<p>No red should be seen.</p>
<div id="parent" style="height:300px; background:green;">
<div id="stf" style="float:left; height:100%; background:red;">
<div id="middle" style="height:100%;">
<img style="display:block; height:100%; background:yellow;"
src='data:image/svg+xml;utf8,<svg height="2px" width="1px"
xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
version="1.1"></svg>'></img>
</div>
</div>
</div>
<script>
document.body.offsetTop;
document.getElementById("parent").style.height = "150px";
</script>
The object that needs to have its preferred logical widths marked dirty is #stf
(it's a float, not an inline-block). The parent of the object that
NeedsPreferredWidthsRecalculation() is #middle.
Yiorsi
What about inline-flex(-webkit-inline-box) or inline-grid?
3 years, 8 months ago
(2017-04-20 05:36:40 UTC)
#10
What about inline-flex(-webkit-inline-box) or inline-grid?
Sunghoon Kim
Description was changed from ========== Make inline-block container's width update Make inline-block container's preferred width ...
3 years, 8 months ago
(2017-04-20 06:50:25 UTC)
#11
Description was changed from
==========
Make inline-block container's width update
Make inline-block container's preferred width update
if its child's preferred width needs to be recalculated.
BUG=707151, 514663
==========
to
==========
Add NeedsPreferredWidthRecalculation() for block
In the case the block has relative height and width is auto and
it has an mage which should keep the aspect ratio,
it needs preferred width recalculation if the parent changes the height.
To cover this, add NeedsPreferredWidthRecalculation() for LayoutBlock.
BUG=707151, 514663
==========
Sunghoon Kim
The problem was, i think, the block which has relative height and width auto was ...
3 years, 8 months ago
(2017-04-20 23:19:14 UTC)
#12
The problem was, i think, the block which has relative height and width auto was
not resized when the parent changed its height if the child of the block had
changed its size.
Here, override NeedsPreferredWidthCalculation() and make preferred width to be
recalculated when the block has relative height and width auto like
LayoutReplaced.
Could you review this CL?
Thanks.
mstensho (USE GERRIT)
This looks like an excellent solution! Before landing, though: Could you proof-read the commit message? ...
3 years, 8 months ago
(2017-04-24 09:08:35 UTC)
#13
This looks like an excellent solution!
Before landing, though:
Could you proof-read the commit message? It's not called
"NeedsPreferredWidthRecalculation()", but rather
"NeedsPreferredWidthsRecalculation()". Also, "mage" -> "image"
Could you also add another, slightly more evil, test, which uses nested blocks
with percentage height, e.g. like in my example in #msg9 .
Sunghoon Kim
Description was changed from ========== Add NeedsPreferredWidthRecalculation() for block In the case the block has ...
3 years, 8 months ago
(2017-04-25 00:21:38 UTC)
#14
Description was changed from
==========
Add NeedsPreferredWidthRecalculation() for block
In the case the block has relative height and width is auto and
it has an mage which should keep the aspect ratio,
it needs preferred width recalculation if the parent changes the height.
To cover this, add NeedsPreferredWidthRecalculation() for LayoutBlock.
BUG=707151, 514663
==========
to
==========
Add NeedsPreferredWidthsRecalculation() for block
In the case the block has relative height and width is auto and
it has an image which should keep the aspect ratio,
it needs preferred width recalculation if the parent changes the height.
To cover this, add NeedsPreferredWidthsRecalculation() for LayoutBlock.
BUG=707151, 514663
==========
Sunghoon Kim
Thanks for your review. I added nested block case in the tc and tc moved ...
3 years, 8 months ago
(2017-04-25 00:23:34 UTC)
#15
Thanks for your review.
I added nested block case in the tc and tc moved to fast/block.
Also, i modified the commit message as you commented.
Any other comments are welcome.
Sunghoon Kim
Description was changed from ========== Add NeedsPreferredWidthsRecalculation() for block In the case the block has ...
3 years, 8 months ago
(2017-04-25 02:50:50 UTC)
#16
Description was changed from
==========
Add NeedsPreferredWidthsRecalculation() for block
In the case the block has relative height and width is auto and
it has an image which should keep the aspect ratio,
it needs preferred width recalculation if the parent changes the height.
To cover this, add NeedsPreferredWidthsRecalculation() for LayoutBlock.
BUG=707151, 514663
==========
to
==========
Add NeedsPreferredWidthsRecalculation() for block
In the case the block has relative height and width is auto and
it has an image which should keep the aspect ratio,
it needs preferred width recalculation if the parent changes the height.
To cover this, add NeedsPreferredWidthsRecalculation() for LayoutBlock.
BUG=707151, 514663, 709233
==========
CQ is committing da patch. Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493107627800340, "parent_rev": "9e8f21af25c5425136583d7658850e4a7db2146e", "commit_rev": "76e307f44292b2999535aa8c5605cfd9129e9c12"}
3 years, 8 months ago
(2017-04-25 09:37:44 UTC)
#22
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493107627800340,
"parent_rev": "9e8f21af25c5425136583d7658850e4a7db2146e", "commit_rev":
"76e307f44292b2999535aa8c5605cfd9129e9c12"}
commit-bot: I haz the power
Description was changed from ========== Add NeedsPreferredWidthsRecalculation() for block In the case the block has ...
3 years, 8 months ago
(2017-04-25 09:37:54 UTC)
#23
Message was sent while issue was closed.
Description was changed from
==========
Add NeedsPreferredWidthsRecalculation() for block
In the case the block has relative height and width is auto and
it has an image which should keep the aspect ratio,
it needs preferred width recalculation if the parent changes the height.
To cover this, add NeedsPreferredWidthsRecalculation() for LayoutBlock.
BUG=707151, 514663, 709233
==========
to
==========
Add NeedsPreferredWidthsRecalculation() for block
In the case the block has relative height and width is auto and
it has an image which should keep the aspect ratio,
it needs preferred width recalculation if the parent changes the height.
To cover this, add NeedsPreferredWidthsRecalculation() for LayoutBlock.
BUG=707151, 514663, 709233
Review-Url: https://codereview.chromium.org/2828453002
Cr-Commit-Position: refs/heads/master@{#466934}
Committed:
https://chromium.googlesource.com/chromium/src/+/76e307f44292b2999535aa8c5605...
==========
commit-bot: I haz the power
Committed patchset #7 (id:120001) as https://chromium.googlesource.com/chromium/src/+/76e307f44292b2999535aa8c5605cfd9129e9c12
3 years, 8 months ago
(2017-04-25 09:37:55 UTC)
#24
Issue 2828453002: Add NeedsPreferredWidthsRecalculation() for block
(Closed)
Created 3 years, 8 months ago by Sunghoon Kim
Modified 3 years, 8 months ago
Reviewers: fs, mstensho (USE GERRIT), Stephen Chennney, rhogan
Base URL:
Comments: 8