Bug 112361
| Summary: | [CSS Grid Layout] computedUsedBreadthOfGridTracks doesn't fill all the grid tracks | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Julien Chaffraix <jchaffraix> |
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | ojan, tony, xan.lopez |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | All | ||
| Bug Depends on: | |||
| Bug Blocks: | 60731 | ||
Julien Chaffraix
The following code can be found in computedUsedBreadthOfGridTracks:
const Vector<GridTrackSize>& trackStyles = (direction == ForColumns) ? style()->gridColumns() : style()->gridRows();
[...]
Vector<GridTrack>& tracks = (direction == ForColumns) ? columnTracks : rowTracks;
for (size_t i = 0; i < trackStyles.size(); ++i) {
GridTrack& track = tracks[i];
const Length& minTrackBreadth = trackStyles[i].minTrackBreadth();
const Length& maxTrackBreadth = trackStyles[i].maxTrackBreadth();
[...]
}
While this will never read past |tracks| end, it's wrong as implicit rows / columns can make tracks.size() > trackStyles.size(). This means that implicit rows / columns are not set up properly prior to running the algorithm.
The loop should actually be:
for (size_t i = 0; i < tracks.size(); ++i) {
GridTrack& track = tracks[i];
const GridTrackSize& trackSize = gridTrackSize(i, direction);
const Length& minTrackBreadth = trackSize.minTrackBreadth();
const Length& maxTrackBreadth = trackSize.maxTrackBreadth();
[...]
As a side note, we should investigate all the call sites iterating over the style's gridRows() / gridColumns() as they are probably wrong.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Julien Chaffraix
I am not sure this bug is visible now. That's because we allow a grid track's usedbreadth to go above its maxbreadth if there is some max-content max sizing function, which matches the specification. This is used e.g. to handle the case where the max sizing function is smaller than the min sizing function.
Once we have implemented the default implicit sizing (bug 103333), it should be trivial to come up with a test case.
Julien Chaffraix
This was fixed when implementing bug 103333 as it was required to pass the tests. The only call site using the style information is RenderGrid::maximumIndexInDirection which is legit as it estimates the grid size based on the style information.