Bug 146021

Summary: min-width/height should default to auto for grid items
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: CSSAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, esprehn+autocc, fpizlo, glenn, hyatt, jfernandez, kling, kondapallykalyan, rego, svillar, zalan
Priority: P2 Keywords: BlinkMergeCandidate
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 146020    
Bug Blocks: 146018    
Attachments:
Description Flags
Patch
none
Rebased patch darin: review+

Sergio Villar Senin
Reported 2015-06-16 08:43:39 PDT
Attachments
Patch (12.65 KB, patch)
2015-09-09 02:54 PDT, Sergio Villar Senin
no flags
Rebased patch (12.52 KB, patch)
2015-09-11 06:46 PDT, Sergio Villar Senin
darin: review+
Sergio Villar Senin
Comment 1 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)
Sergio Villar Senin
Comment 2 2015-09-09 01:43:51 PDT
Sergio Villar Senin
Comment 3 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.
Sergio Villar Senin
Comment 4 2015-09-09 02:54:20 PDT
Sergio Villar Senin
Comment 5 2015-09-11 06:46:10 PDT
Created attachment 261005 [details] Rebased patch
Darin Adler
Comment 6 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>.
Sergio Villar Senin
Comment 7 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.
Sergio Villar Senin
Comment 8 2015-09-14 03:41:16 PDT
Note You need to log in before you can comment on or make changes to this bug.