Bug 148645 - [CSS Grid Layout] Layout is wrong for flex factor sum between 0 and 1
Summary: [CSS Grid Layout] Layout is wrong for flex factor sum between 0 and 1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Javier Fernandez
URL:
Keywords: BlinkMergeCandidate
Depends on:
Blocks:
 
Reported: 2015-08-31 13:52 PDT by Javier Fernandez
Modified: 2015-09-01 08:04 PDT (History)
9 users (show)

See Also:


Attachments
test to reproduce the issue (783 bytes, text/html)
2015-08-31 13:52 PDT, Javier Fernandez
no flags Details
Patch (7.69 KB, patch)
2015-09-01 03:44 PDT, Javier Fernandez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Javier Fernandez 2015-08-31 13:52:32 PDT
Created attachment 260313 [details]
test to reproduce the issue

Steps to reproduce the problem:
1. load the attached HTML file
2. 
3. 

What is the expected behavior?
The columns in the 2nd grid should only extend to about 60%
(0.1 + 0.2 + 0.3) of the page width.

What went wrong?
The two grid containers renders exactly the same which demonstrates that Chrome scales up the flex sizes so that their sum is 1.  This is wrong per the Grid spec.
https://drafts.csswg.org/css-grid/#algo-flex-tracks
Comment 1 Javier Fernandez 2015-08-31 13:52:57 PDT
In particular, the correct result is achieved by <https://drafts.csswg.org/css-grid/#algo-find-fr-size>, step 2, second sentence. If you don't do this, you'll normalize sums less than 1.

This is necessary to do for the same reason as it is in Flexbox - to make transitions/animations work correctly and smoothly to/from 0.
Comment 2 Javier Fernandez 2015-08-31 13:53:43 PDT
You need to actually step through that algorithm and see what that step does.  "Set it to 1 instead" has a very specific effect that's not intuitive if you haven't seen it before - it ensures that we *don't* normalize the individual tracks to fill the space.

More precisely, when we sum up the flexes, that tells us how many "units" of flex need to fit into the free space. We then figure out what size to assign to the flex unit based on that, and send that back to the individual items, sizing them appropriately to their flex value.

If you *naively* use the sum in all cases, then in a case like ".1fr .2fr .3fr", you tell the algorithm "I've got .6 units of flex to fit into that space. Tell me how big 1fr should be to make that happen", and the algorithm spits out a size that makes all the columns fill the space.  If you do step 2 *correctly*, you lie and say "I've got 1 unit of flex to fit into that space". Then when you get the size of a flex unit, the columns are sized accordingly, and only fill 60% of the space.

This is the correct behavior.  For example, it lets you animate from 1fr to 0fr correctly.  The naive behavior would make the track fill the entire space all the way from 1 to 0+ε, then we'd need a special case to handle 0 flex to avoid a division by 0.  The specced behavior instead makes it shrink to zero smoothly, and handles the zero case automatically.
Comment 3 Javier Fernandez 2015-09-01 03:44:24 PDT
Created attachment 260359 [details]
Patch
Comment 4 WebKit Commit Bot 2015-09-01 08:04:32 PDT
Comment on attachment 260359 [details]
Patch

Clearing flags on attachment: 260359

Committed r189208: <http://trac.webkit.org/changeset/189208>
Comment 5 WebKit Commit Bot 2015-09-01 08:04:36 PDT
All reviewed patches have been landed.  Closing bug.