**What steps will reproduce the problem?** Load this URL data:text/html,<div id="foo" style="grid-template-columns: [a] repeat(auto-fill, 10px [b]) [c]"></div><script>document.write(getComputedStyle(foo).gridTemplateColumns);</script> **What is the expected result?** You get `[a] repeat(auto-fill, 10px [b]) [c]`. According to https://drafts.csswg.org/css-grid/#resolved-track-list, the resolved value is the computed value for non-grid displays. **What happens instead?** You get `[a]`. Or a failed assert crash in debug builds. Has been fixed in Blink: https://crbug.com/988516
Created attachment 379626 [details] Patch
Created attachment 379627 [details] Patch
Created attachment 379629 [details] Patch
Comment on attachment 379629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379629&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1257 > + for (unsigned i = start; i < end; ++i) { Wouldn't be equivalent, and clearer, to initialize 'i' to 'start + offset' ? > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1297 > + populateGridTrackList(list, collector, grid->trackSizesForComputedStyle(direction), [&](const LayoutUnit& v) { I think 'list' has Ref<CSSValueList> as type, so either populateGridTrackList argument receives a Ref as well and you transfer the ownership or just use a non ref-counted type, like the addValuesForNamedGridLinesAtIndex function does; I think this is the right approach, since populateGridTrackList doesn't need to keep or create a new object reference. In that case, you use the 'get' function to get the raw pointer reference. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1311 > + populateGridTrackList(list, collector, trackSizes, getTrackSize); Ditto > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1317 > + populateGridTrackList(list, collector, trackSizes, getTrackSize, 0, autoRepeatInsertionPoint); Ditto > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1323 > + populateGridTrackList(*repeatedValues, repeatCollector, autoRepeatTrackSizes, getTrackSize); Ditto > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1327 > + populateGridTrackList(list, collector, trackSizes, getTrackSize, autoRepeatInsertionPoint, trackSizes.size(), 1); Ditto
Comment on attachment 379629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379629&action=review >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1257 >> + for (unsigned i = start; i < end; ++i) { > > Wouldn't be equivalent, and clearer, to initialize 'i' to 'start + offset' ? But then I would need tracks[i - offset] >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1297 >> + populateGridTrackList(list, collector, grid->trackSizesForComputedStyle(direction), [&](const LayoutUnit& v) { > > I think 'list' has Ref<CSSValueList> as type, so either populateGridTrackList argument receives a Ref as well and you transfer the ownership or just use a non ref-counted type, like the addValuesForNamedGridLinesAtIndex function does; I think this is the right approach, since populateGridTrackList doesn't need to keep or create a new object reference. In that case, you use the 'get' function to get the raw pointer reference. OK I will do these changes: I will do these changes: diff --git a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp b/Source/WebCore/css/CSSComputedStyleDeclaration.cpp index d55eddb56b..5806fac7b2 100644 --- a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp +++ b/Source/WebCore/css/CSSComputedStyleDeclaration.cpp @@ -1294,7 +1294,7 @@ static Ref<CSSValue> valueForGridTrackList(GridTrackSizingDirection direction, R if (isRenderGrid) { auto* grid = downcast<RenderGrid>(renderer); OrderedNamedLinesCollectorInGridLayout collector(style, isRowAxis, grid->autoRepeatCountForDirection(direction)); - populateGridTrackList(list, collector, grid->trackSizesForComputedStyle(direction), [&](const LayoutUnit& v) { + populateGridTrackList(list.get(), collector, grid->trackSizesForComputedStyle(direction), [&](const LayoutUnit& v) { return zoomAdjustedPixelValue(v, style); }); return list; @@ -1308,23 +1308,23 @@ static Ref<CSSValue> valueForGridTrackList(GridTrackSizingDirection direction, R if (autoRepeatTrackSizes.isEmpty()) { // If there's no auto repeat(), just add all the line names and track sizes. - populateGridTrackList(list, collector, trackSizes, getTrackSize); + populateGridTrackList(list.get(), collector, trackSizes, getTrackSize); return list; } // Add the line names and track sizes that precede the auto repeat(). unsigned autoRepeatInsertionPoint = isRowAxis ? style.gridAutoRepeatColumnsInsertionPoint() : style.gridAutoRepeatRowsInsertionPoint(); - populateGridTrackList(list, collector, trackSizes, getTrackSize, 0, autoRepeatInsertionPoint); + populateGridTrackList(list.get(), collector, trackSizes, getTrackSize, 0, autoRepeatInsertionPoint); // Add a CSSGridAutoRepeatValue with the contents of the auto repeat(). AutoRepeatType autoRepeatType = isRowAxis ? style.gridAutoRepeatColumnsType() : style.gridAutoRepeatRowsType(); - RefPtr<CSSValueList> repeatedValues = CSSGridAutoRepeatValue::create(autoRepeatType == AutoRepeatType::Fill ? CSSValueAutoFill : CSSValueAutoFit); + auto repeatedValues = CSSGridAutoRepeatValue::create(autoRepeatType == AutoRepeatType::Fill ? CSSValueAutoFill : CSSValueAutoFit); OrderedNamedLinesCollectorInsideRepeat repeatCollector(style, isRowAxis); - populateGridTrackList(*repeatedValues, repeatCollector, autoRepeatTrackSizes, getTrackSize); - list->append(repeatedValues.releaseNonNull()); + populateGridTrackList(repeatedValues.get(), repeatCollector, autoRepeatTrackSizes, getTrackSize); + list->append(repeatedValues.get()); // Add the line names and track sizes that follow the auto repeat(). - populateGridTrackList(list, collector, trackSizes, getTrackSize, autoRepeatInsertionPoint, trackSizes.size(), 1); + populateGridTrackList(list.get(), collector, trackSizes, getTrackSize, autoRepeatInsertionPoint, trackSizes.size(), 1); return list; }
Created attachment 380101 [details] Patch
Comment on attachment 380101 [details] Patch Clearing flags on attachment: 380101 Committed r250715: <https://trac.webkit.org/changeset/250715>
All reviewed patches have been landed. Closing bug.
<rdar://problem/55978112>