Bug 146021 - min-width/height should default to auto for grid items
Summary: min-width/height should default to auto for grid items
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: BlinkMergeCandidate
Depends on: 146020
Blocks: 146018
  Show dependency treegraph
 
Reported: 2015-06-16 08:43 PDT by Sergio Villar Senin
Modified: 2015-09-14 03:41 PDT (History)
12 users (show)

See Also:


Attachments
Patch (12.65 KB, patch)
2015-09-09 02:54 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Rebased patch (12.52 KB, patch)
2015-09-11 06:46 PDT, Sergio Villar Senin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2015-06-16 08:43:39 PDT
As stated here http://dev.w3.org/csswg/css-grid/#min-size-auto
Comment 1 Sergio Villar Senin 2015-06-16 08:44:56 PDT
We need to merge the following Blink changes:


- https://crrev.com/194408 
- https://crrev.com/194863
- https://crrev.com/194887 (partially)
Comment 2 Sergio Villar Senin 2015-09-09 01:43:51 PDT
Also this one https://src.chromium.org/viewvc/blink?view=revision&revision=201797
Comment 3 Sergio Villar Senin 2015-09-09 02:41:27 PDT
(In reply to comment #2)
> Also this one
> https://src.chromium.org/viewvc/blink?view=revision&revision=201797

We'll merge that later once we have the alignment logic in place.
Comment 4 Sergio Villar Senin 2015-09-09 02:54:20 PDT
Created attachment 260852 [details]
Patch
Comment 5 Sergio Villar Senin 2015-09-11 06:46:10 PDT
Created attachment 261005 [details]
Rebased patch
Comment 6 Darin Adler 2015-09-11 09:01:18 PDT
Comment on attachment 261005 [details]
Rebased patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261005&action=review

> Source/WebCore/rendering/RenderBox.cpp:2757
> +                if (Optional<LayoutUnit> minContentHeight = computeContentLogicalHeight(MinSize, Length(MinContent), computedValues.m_extent - borderAndPaddingLogicalHeight()))
> +                    contentHeight = std::max(contentHeight, constrainLogicalHeightByMinMax(minContentHeight.value(), computedValues.m_extent - borderAndPaddingLogicalHeight()));

Put the repeated expression computedValues.m_extent - borderAndPaddingLogicalHeight() into a local variable?

Might also want to use auto instead of explicitly stating Optional<LayoutUnit>.
Comment 7 Sergio Villar Senin 2015-09-14 00:21:08 PDT
Comment on attachment 261005 [details]
Rebased patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261005&action=review

>> Source/WebCore/rendering/RenderBox.cpp:2757
>> +                    contentHeight = std::max(contentHeight, constrainLogicalHeightByMinMax(minContentHeight.value(), computedValues.m_extent - borderAndPaddingLogicalHeight()));
> 
> Put the repeated expression computedValues.m_extent - borderAndPaddingLogicalHeight() into a local variable?
> 
> Might also want to use auto instead of explicitly stating Optional<LayoutUnit>.

Sure, I'll do.
Comment 8 Sergio Villar Senin 2015-09-14 03:41:16 PDT
Committed r189708: <http://trac.webkit.org/changeset/189708>