WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
149876
[css-grid] Include freeSpace in GridSizingData struct
https://bugs.webkit.org/show_bug.cgi?id=149876
Summary
[css-grid] Include freeSpace in GridSizingData struct
Sergio Villar Senin
Reported
2015-10-07 08:31:25 PDT
We're passing freeSpaceForRows and freeSpaceForColumns to a bunch of functions along with the GridSizingData struct when we should really include them both in the struct as we're talking about temporary data that is only used for the layout process.
Attachments
Patch
(12.44 KB, patch)
2015-10-07 08:46 PDT
,
Sergio Villar Senin
no flags
Details
Formatted Diff
Diff
Patch
(12.49 KB, patch)
2015-10-08 05:20 PDT
,
Sergio Villar Senin
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Sergio Villar Senin
Comment 1
2015-10-07 08:46:06 PDT
Created
attachment 262610
[details]
Patch
Darin Adler
Comment 2
2015-10-07 09:11:18 PDT
Comment on
attachment 262610
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=262610&action=review
Need a new patch; looks like this one doesn’t build.
> Source/WebCore/rendering/RenderGrid.cpp:214 > + GridSizingData(unsigned gridColumnCount, unsigned gridRowCount, const LayoutUnit& freeSpaceForColumns, const LayoutUnit& freeSpaceForRows)
LayoutUnit is small enough that we probably want to just pass LayoutUnit, not const LayoutUnit&.
> Source/WebCore/rendering/RenderGrid.cpp:358 > + const_cast<RenderGrid*>(this)->computeUsedBreadthOfGridTracks(ForColumns, sizingData);
The const_cast in this function is strange. Doesn’t seem like the right pattern for functions with side effects. Not new to this patch, though.
Sergio Villar Senin
Comment 3
2015-10-08 05:17:15 PDT
(In reply to
comment #2
)
> Comment on
attachment 262610
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=262610&action=review
> > Need a new patch; looks like this one doesn’t build.
Beh, I somehow committed a patch without one of the replacements.
Sergio Villar Senin
Comment 4
2015-10-08 05:20:45 PDT
Created
attachment 262688
[details]
Patch
Sergio Villar Senin
Comment 5
2015-10-09 05:57:21 PDT
Committed
r190784
: <
http://trac.webkit.org/changeset/190784
>
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