WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
214315
[css-grid] CSSOM gridTemplateColumns/Rows reports wrong line names when leading implicit tracks are present
https://bugs.webkit.org/show_bug.cgi?id=214315
Summary
[css-grid] CSSOM gridTemplateColumns/Rows reports wrong line names when leadi...
Oriol Brufau
Reported
2020-07-14 13:08:12 PDT
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.
Attachments
Patch
(30.93 KB, patch)
2020-07-14 13:31 PDT
,
Oriol Brufau
no flags
Details
Formatted Diff
Diff
Patch
(20.63 KB, patch)
2020-07-15 05:27 PDT
,
Oriol Brufau
no flags
Details
Formatted Diff
Diff
Patch
(20.57 KB, patch)
2020-07-15 14:17 PDT
,
Oriol Brufau
no flags
Details
Formatted Diff
Diff
Patch
(20.62 KB, patch)
2020-07-16 05:33 PDT
,
Oriol Brufau
no flags
Details
Formatted Diff
Diff
Patch
(20.64 KB, patch)
2020-07-16 09:33 PDT
,
Oriol Brufau
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Oriol Brufau
Comment 1
2020-07-14 13:31:17 PDT
Created
attachment 404272
[details]
Patch
Manuel Rego Casasnovas
Comment 2
2020-07-15 01:07:53 PDT
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?
Oriol Brufau
Comment 3
2020-07-15 05:09:21 PDT
(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
.
Oriol Brufau
Comment 4
2020-07-15 05:27:53 PDT
Created
attachment 404335
[details]
Patch
Manuel Rego Casasnovas
Comment 5
2020-07-15 13:56:28 PDT
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.
Oriol Brufau
Comment 6
2020-07-15 14:17:03 PDT
Created
attachment 404393
[details]
Patch
Manuel Rego Casasnovas
Comment 7
2020-07-16 00:49:31 PDT
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()));
Oriol Brufau
Comment 8
2020-07-16 05:33:40 PDT
Created
attachment 404435
[details]
Patch
Oriol Brufau
Comment 9
2020-07-16 09:33:55 PDT
Created
attachment 404451
[details]
Patch
EWS
Comment 10
2020-07-16 09:59:23 PDT
Committed
r264465
: <
https://trac.webkit.org/changeset/264465
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 404451
[details]
.
Radar WebKit Bug Importer
Comment 11
2020-07-16 10:00:24 PDT
<
rdar://problem/65672557
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug