Bug 113610 - intrinsic size keywords don't work for heights
Summary: intrinsic size keywords don't work for heights
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Robert Hogan
Keywords: BlinkMergeCandidate
Depends on:
Blocks: 146018
  Show dependency treegraph
Reported: 2013-03-29 14:36 PDT by Ojan Vafai
Modified: 2017-08-22 19:47 PDT (History)
16 users (show)

See Also:

Patch (74.36 KB, patch)
2015-06-11 07:15 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 Ojan Vafai 2013-03-29 14:36:17 PDT

Right now we don't even parse them.
Comment 1 Christian Biesinger 2013-05-20 14:51:58 PDT
FYI, the blink version of this code was:

Note the followup in https://codereview.chromium.org/15112004/
Comment 2 Sergio Villar Senin 2015-06-11 07:15:58 PDT
Created attachment 254721 [details]
Comment 3 Sergio Villar Senin 2015-06-23 02:38:21 PDT
Pinging reviewers...
Comment 4 Darin Adler 2015-06-23 10:09:20 PDT
Comment on attachment 254721 [details]

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

Not at all fond of using the -1 magic value here.

> Source/WebCore/css/CSSParser.cpp:1817
> +bool CSSParser::isValidSize(ValueWithCalculation& valueWithCalculation)

If this has no side effects, maybe the argument type should be const& instead of just &.

> Source/WebCore/rendering/RenderBox.cpp:2819
> +    // FIXME: the css-sizing spec is considering changing what min-content/max-content should resolve to.

Should be sentence style. Also should be called CSS sizing, not css-sizing.

> Source/WebCore/rendering/RenderTable.cpp:374
> +    } else if (styleLogicalHeight.isPercentOrCalculated()) {
>          computedLogicalHeight = computePercentageLogicalHeight(styleLogicalHeight);
> +    } else if (styleLogicalHeight.isIntrinsic())

WebKit code says no braces for this kind of thing.
Comment 5 Sergio Villar Senin 2015-06-24 06:37:02 PDT
Committed r185908: <http://trac.webkit.org/changeset/185908>
Comment 6 Basile Clement 2015-07-29 10:55:02 PDT
Removed LayoutTests/fast/css-intrinsic-dimensions/height-property-value.html.orig in http://trac.webkit.org/changeset/187549