Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(873)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 360613003: Ensure we compute the min and max height of replaced elements to 'none' or 0 when appropriate. (Closed)

Created:
6 years, 5 months ago by rhogan
Modified:
6 years, 5 months ago
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr., rune+blink, zoltan1
Project:
blink
Visibility:
Public.

Description

Ensure we compute the min and max height of replaced elements to 'none' or 0 when appropriate. If a replaced element has a percentage min or max height specified then that height value should compute to 'none' for max-height and 0 for min-height when its containing block does not have a height 'specified explicitly'. See http://www.w3.org/TR/CSS21/visudet.html#min-max-heights http://lists.w3.org/Archives/Public/www-style/2014Jun/0079.html BUG=385877 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178397

Patch Set 1 #

Total comments: 6

Patch Set 2 : Updated per Commetns #

Total comments: 4

Patch Set 3 : Updated #

Unified diffs Side-by-side diffs Delta from patch set Stats (+125 lines, -2 lines) Patch
A LayoutTests/css2.1/20110323/max-height-percentage-003.html View 1 1 chunk +43 lines, -0 lines 0 comments Download
A LayoutTests/css2.1/20110323/max-height-percentage-003-expected.html View 1 1 chunk +6 lines, -0 lines 0 comments Download
A LayoutTests/fast/replaced/max-height-percentage-quirks.html View 1 1 chunk +8 lines, -0 lines 0 comments Download
A LayoutTests/fast/replaced/max-height-percentage-quirks-expected.html View 1 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/fast/replaced/min-height-percentage.html View 1 1 chunk +9 lines, -0 lines 0 comments Download
A LayoutTests/fast/replaced/min-height-percentage-expected.html View 1 1 chunk +9 lines, -0 lines 0 comments Download
A LayoutTests/fast/replaced/min-height-percentage-quirks.html View 1 1 chunk +8 lines, -0 lines 0 comments Download
A LayoutTests/fast/replaced/min-height-percentage-quirks-expected.html View 1 1 chunk +8 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderBox.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 2 1 chunk +29 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
leviw_travelin_and_unemployed
6 years, 5 months ago (2014-07-07 17:55:53 UTC) #1
leviw_travelin_and_unemployed
https://codereview.chromium.org/360613003/diff/1/Source/core/rendering/RenderBox.cpp File Source/core/rendering/RenderBox.cpp (right): https://codereview.chromium.org/360613003/diff/1/Source/core/rendering/RenderBox.cpp#newcode2782 Source/core/rendering/RenderBox.cpp:2782: if (!logicalHeightLength.isPercent() || isOutOfFlowPositioned() || document().inQuirksMode()) Why the Quirks ...
6 years, 5 months ago (2014-07-07 18:02:35 UTC) #2
rhogan
On 2014/07/07 at 18:02:35, leviw wrote: > https://codereview.chromium.org/360613003/diff/1/Source/core/rendering/RenderBox.cpp > File Source/core/rendering/RenderBox.cpp (right): > > https://codereview.chromium.org/360613003/diff/1/Source/core/rendering/RenderBox.cpp#newcode2782 ...
6 years, 5 months ago (2014-07-07 18:10:32 UTC) #3
esprehn
https://codereview.chromium.org/360613003/diff/1/LayoutTests/css2.1/20110323/max-height-percentage-003-expected.html File LayoutTests/css2.1/20110323/max-height-percentage-003-expected.html (right): https://codereview.chromium.org/360613003/diff/1/LayoutTests/css2.1/20110323/max-height-percentage-003-expected.html#newcode1 LayoutTests/css2.1/20110323/max-height-percentage-003-expected.html:1: <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd"> Lets use ...
6 years, 5 months ago (2014-07-07 18:15:53 UTC) #4
rhogan
On 2014/07/07 at 18:15:53, esprehn wrote: > > https://codereview.chromium.org/360613003/diff/1/Source/core/rendering/RenderBox.cpp#newcode2798 > Source/core/rendering/RenderBox.cpp:2798: LayoutUnit minLogicalHeight = logicalHeightComputesAsNone(MinSize) ...
6 years, 5 months ago (2014-07-09 19:49:13 UTC) #5
esprehn
https://codereview.chromium.org/360613003/diff/20001/Source/core/rendering/RenderBox.cpp File Source/core/rendering/RenderBox.cpp (right): https://codereview.chromium.org/360613003/diff/20001/Source/core/rendering/RenderBox.cpp#newcode2778 Source/core/rendering/RenderBox.cpp:2778: Length logicalHeightLength = sizeType == MinSize ? style()->logicalMinHeight() : ...
6 years, 5 months ago (2014-07-16 20:43:50 UTC) #6
rhogan
On 2014/07/16 at 20:43:50, esprehn wrote: > https://codereview.chromium.org/360613003/diff/20001/Source/core/rendering/RenderBox.cpp > File Source/core/rendering/RenderBox.cpp (right): > Thanks esprehn ...
6 years, 5 months ago (2014-07-17 19:00:41 UTC) #7
esprehn
On 2014/07/17 19:00:41, rhogan wrote: > On 2014/07/16 at 20:43:50, esprehn wrote: > > > ...
6 years, 5 months ago (2014-07-17 19:02:40 UTC) #8
rhogan
The CQ bit was checked by robhogan@gmail.com
6 years, 5 months ago (2014-07-17 19:08:54 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/robhogan@gmail.com/360613003/40001
6 years, 5 months ago (2014-07-17 19:10:30 UTC) #10
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_rel on tryserver.blink ...
6 years, 5 months ago (2014-07-17 20:01:45 UTC) #11
commit-bot: I haz the power
6 years, 5 months ago (2014-07-17 20:47:02 UTC) #12
Message was sent while issue was closed. 
Change committed as 178397

Powered by Google App Engine
This is Rietveld 408576698