These WPT are failing: imported/w3c/web-platform-tests/css/css-grid/parsing/grid-template-columns-computed-implicit-track.html imported/w3c/web-platform-tests/css/css-grid/parsing/grid-template-rows-computed-implicit-track.html They were added in https://crbug.com/790988, the Chromium patch needs to be ported to WebKit.
Created attachment 404272 [details] Patch
Comment on attachment 404272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404272&action=review On a quick look, it seems good. But I'd split it in 2 patches (one for the refactoring and another for the actual fix). Thanks! > Source/WebCore/ChangeLog:20 > + This patch also renames SmallestTrackStart to ExplicitGridStart. > + The SmallestTrackStart method used to return a signed int which was > + never positive, representing the smallest untranslated start among all > + grid items, clamped by 0 as a maximum. Can we do this change on a different patch? This is basically a refactoring and should have no behavior changes, so we can land that one first, and then the actual fix for this bug. WDYT?
(In reply to Manuel Rego Casasnovas from comment #2) > This is basically a refactoring and should have no behavior changes, so we > can land that one first, and then the actual fix for this bug. WDYT? OK, filed bug 214347.
Created attachment 404335 [details] Patch
Comment on attachment 404335 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404335&action=review Thanks for the fix. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:875 > + ASSERT((unsigned)end <= tracks.size()); Do we need this cast? > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:920 > + // Named grid line indices are relative to the explicit grid, but we are > + // including all tracks. So we need to subtract the number of leading > + // implicit tracks in order to get the proper line index. Nit: Comments lines can be longer in WebKit, no need to wrap to 80-chars.
Created attachment 404393 [details] Patch
Comment on attachment 404393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404393&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:875 > ASSERT(end <= tracks.size()); So it looks like the cast was actually needed: ./css/CSSComputedStyleDeclaration.cpp:875:16: error: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare] ASSERT(end <= tracks.size()); Probably you can do something like: ASSERT(end <= static_cast<int>(tracks.size()));
Created attachment 404435 [details] Patch
Created attachment 404451 [details] Patch
Committed r264465: <https://trac.webkit.org/changeset/264465> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404451 [details].
<rdar://problem/65672557>