New issue
Advanced search Search tips

Issue 675333 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

max-width calculates incorrect width when using flex-direction: column

Reported by kybis...@gmail.com, Dec 17 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.95 Safari/537.36

Steps to reproduce the problem:
1. Add `display: flex; flex-direction: column` to a container div
2. Add `display: flex; margin-left: 50%; max-width: 50%` to a child div

What is the expected behavior?
The child div should have width 50% of its parent

What went wrong?
The child's margin is subtracted from the container width before the child width is calculated.

The child width is now `(containerWidth - childMargin) * max-width = (100% - 50%) * 50% = 25%`

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 55.0.2883.95  Channel: stable
OS Version: OS X 10.12.1
Flash Version: Shockwave Flash 24.0 r0

`max-width` calculates width correctly if `flex-direction` is set to `row`.
`width` still works correctly regardless of `flex-direction`

This behavior is also broken in Safari, but works correctly in Firefox.
 
flexDirectionBug.html
382 bytes View Download

Comment 1 by kybis...@gmail.com, Dec 17 2016

jsfiddle of the bug: https://jsfiddle.net/kbkhpx2n/

Comment 2 by tkent@chromium.org, Dec 18 2016

Components: -Blink>CSS Blink>Layout>Flexbox

Comment 3 by ajha@chromium.org, Dec 19 2016

Labels: M-55
Owner: cbiesin...@chromium.org
Status: Assigned (was: Unconfirmed)
Huh, indeed, the code looks wrong:
    LayoutUnit childWidth =
        (lineCrossAxisExtent - crossAxisMarginExtentForChild(child))
            .clampNegativeToZero();
    childWidth =
        child.constrainLogicalWidthByMinMax(childWidth, childWidth, this);

The second childWidth should instead be the available width.
Looks like the code has been the same ever since it was introduced in ccb67766f30924441231bcb188fb91c1fff9290a (https://bugs.webkit.org/show_bug.cgi?id=94604)

Comment 7 by phistuck@gmail.com, Dec 20 2016

#5 - can you report this (or submit a patch) to WebKit as well? :)
https://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp#L1499
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1691bb109535d456e52402b7fd2a82f02c1ba853

commit 1691bb109535d456e52402b7fd2a82f02c1ba853
Author: cbiesinger <cbiesinger@chromium.org>
Date: Tue Dec 20 12:21:01 2016

[css-flexbox] Pass correct available width to constrainLogicalWidthByMinMax

When stretching we used to pass the wrong available width to
constrainLogicalWidthByMinMax, leading to an incorrect calculation of
percentage max/min widths.

BUG= 675333 
R=mstensho@opera.com,eae@chromium.org

Review-Url: https://codereview.chromium.org/2585423002
Cr-Commit-Position: refs/heads/master@{#439772}

[add] https://crrev.com/1691bb109535d456e52402b7fd2a82f02c1ba853/third_party/WebKit/LayoutTests/css3/flexbox/percentage-max-width-cross-axis.html
[modify] https://crrev.com/1691bb109535d456e52402b7fd2a82f02c1ba853/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp

Labels: Merge-Request-56
Status: Fixed (was: Assigned)
Filed https://bugs.webkit.org/show_bug.cgi?id=166061

Also, since this is a very straightforward bugfix, requesting merge.

Comment 10 by dimu@chromium.org, Dec 21 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 21 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40a286ebdbf149c1f7f8cb92fdb91ab46d1a0ace

commit 40a286ebdbf149c1f7f8cb92fdb91ab46d1a0ace
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Wed Dec 21 13:36:34 2016

[css-flexbox] Pass correct available width to constrainLogicalWidthByMinMax

When stretching we used to pass the wrong available width to
constrainLogicalWidthByMinMax, leading to an incorrect calculation of
percentage max/min widths.

BUG= 675333 
R=mstensho@opera.com,eae@chromium.org

Review-Url: https://codereview.chromium.org/2585423002
Cr-Commit-Position: refs/heads/master@{#439772}
(cherry picked from commit 1691bb109535d456e52402b7fd2a82f02c1ba853)

Review-Url: https://codereview.chromium.org/2590163005 .
Cr-Commit-Position: refs/branch-heads/2924@{#576}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[add] https://crrev.com/40a286ebdbf149c1f7f8cb92fdb91ab46d1a0ace/third_party/WebKit/LayoutTests/css3/flexbox/percentage-max-width-cross-axis.html
[modify] https://crrev.com/40a286ebdbf149c1f7f8cb92fdb91ab46d1a0ace/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp

Labels: TE-Verified-56 TE-Verified-56.0.2924.51
Tested the issue on windows 7  using chrome version#56.0.2924.51 with the steps mentioned in comment #0.

Working as expected as per the below fiddle.

https://jsfiddle.net/kbkhpx2n/

Please find the attached screen cast for the same.

Adding TE-Verified labels.
675333.mp4
1.7 MB View Download

Sign in to add a comment