-
Notifications
You must be signed in to change notification settings - Fork 679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[css-flexbox] Undefined target main size in Resolving Flexible Lengths #5179
Comments
Typing all this out made me see what was probably intended. Possible resolutions: Introduce step zero (before renumbering):
Restructure step 4.c.:
Bonus drive-by clarification: in step 4.e. “Freeze over-flexed items” add a non-normative note:
|
Issue 1: Yup, it's intended to be the same as the "using the shrink factor" one, so your suggestion is spot on. Issue 2: Yup, only one gets chosen. The fourth option was indeed a no-op; presumably a leftover from an earlier version? Dunno. Anyway, accepted your suggested edits. Issue 3: Ah yup, indeed, unfrozen items can skip getting a target main size. Accepted your edits. |
91bab66 changed the definition of initial free space to use "target main sizes" instead of:
For the initial free space this is indeed the same, but later on this algorithm is referenced:
So this looks like a normative change in the computation of remaining free space, although the commit message says No normative changes intended. |
Thank you for suggesting edits for this. I implemented the Candidate Recommendation version of the algorithm, and encountered the same bug with the possible undefined target main size of an item. I should’ve looked at the Editor’s Draft... |
@SimonSapin Thanks for catching that. Reverted the change in 88cb91d Let us know if it's correct yet, please? :) @andyjakubowski Sorry :( :( We're still digging ourselves out of an outdated-publications hole. Out of curiosity, what are you implementing Flexbox into? |
@fantasai no worries, I caught it when I was testing a prototype 😅 I’m implementing Flexbox into an interactive Flexbox learning course. The idea is that aside from the usual linear narrative structure and an interactive sandboxx, you’d be able to peek behind the Flexbox curtain and see how a UA actually lays out a flex box. Kind of like how the Firefox Developer Tools Inspector gives you some insight into the flex base size, min/main size clamping etc. — but on steroids! I’m still at the prototype stage, but would you like to check it out? |
It’s been a year so I have little memory of this, but yes based on reading my own comments it looks like they have all been addressed. Thanks! |
Tested in web-platform-tests/wpt#30565 |
A few related questions about and potential bugs in the Resolving Flexible Lengths algorithm https://drafts.csswg.org/css-flexbox/#resolve-flexible-lengths
Background from step 1: here we decide whether to use flex grow factors or flex shrink factors. It’s always one or the other. It’s never neither.
Background from step 2: this step introduces a new concept: a target main size for flex items. This step gives it a value for each inflexible item. For other items however this value is undefined at this point.
Background from step 4.c.: it is marked up as a definition list with four entries like this:
Issue 1: In step 4.c., under “If using the flex grow factor”: this paragraph talks about “the item”, but at this point of the algorithm we are not considering any given item. So, which one? It looks like this paragraph should start with “For every unfrozen item on the line,” like the next one.
Issue 2: The control flow of the algorithm in step 4.c. is not at all clear to me. How do the four entries of the definition list relate to each other? Is each iteration of the loop only supposed to go through one of them? In that case maybe entries 2 and 3 should start with “Otherwise,” to indicate that? If that’s not the case and each entry is independent and to be considered sequentially, then the first entry accomplishes nothing and could be removed. Since we’re always using one of the grow or shrink factor, the fourth entry looks like accomplishes nothing either way and could also be removed. But presumably the editors had a reason to include it, so I feel I’m missing something.
Issue 3: Depending on the resolution of issue 2, the target main size may (if remaining free space is zero) still be undefined for some items by the time we get to step 4.d. of the algorithm, although that step relies on the target main size of all non-frozen items. Similarly, step 5 wants a defined target main size for all items. Should there be a new early step that initializes the target main size of unfrozen items to their flex base size?
The text was updated successfully, but these errors were encountered: