Bug 157971 - [css-grid] Horizontal scroll must account for grid container's height
Summary: [css-grid] Horizontal scroll must account for grid container's height
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Javier Fernandez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-21 00:43 PDT by Javier Fernandez
Modified: 2016-06-06 05:10 PDT (History)
10 users (show)

See Also:

Attachments
Test case to reproduce the crash. (142 bytes, text/html)
2016-05-21 00:43 PDT, Javier Fernandez 		no flags Details
Patch (4.30 KB, patch)
2016-05-23 05:24 PDT, Javier Fernandez 		no flags Details | Formatted Diff | Diff
Patch (4.46 KB, patch)
2016-05-24 11:00 PDT, Javier Fernandez 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-cq-01 for mac-yosemite (966.75 KB, application/zip)
2016-05-24 11:36 PDT, WebKit Commit Bot 		no flags Details
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Javier Fernandez 2016-05-21 00:43:46 PDT 
Created attachment 279540 [details]
Test case to reproduce the crash.

It seems grid layout logic does not consider the scrollbar size when computing the grid container's height. 
This may result in an incorrect sizing when using indefinite sized containers.

Besides, it's possible to make the layout logic to crash due to the fact that contentLogicalHeight can be a negative value if we render a case where content size is 0px.
Comment 1 Javier Fernandez 2016-05-23 05:24:20 PDT 
Created attachment 279562 [details]
Patch
Comment 2 Manuel Rego Casasnovas 2016-05-23 06:11:50 PDT 
Comment on attachment 279562 [details]
Patch

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

LGTM.

> LayoutTests/fast/css-grid-layout/grid-container-scroll-accounts-for-sizing-expected.html:7
> +   width: -webkit-fit-content;

Nit: Maybe you can use "css-intrinsic-dimensions/resources/width-keyword-classes.css".

Or simply something like "position: absolute" or "float: left" to force the shrink-to-fit.
Comment 3 Javier Fernandez 2016-05-24 11:00:32 PDT 
Created attachment 279672 [details]
Patch
Comment 4 WebKit Commit Bot 2016-05-24 11:36:24 PDT 
Comment on attachment 279672 [details]
Patch

Rejecting attachment 279672 [details] from commit-queue.

Number of test failures exceeded the failure limit.
Full output: http://webkit-queues.webkit.org/results/1375984
Comment 5 WebKit Commit Bot 2016-05-24 11:36:28 PDT 
Created attachment 279678 [details]
Archive of layout-test-results from webkit-cq-01 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-01  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 6 WebKit Commit Bot 2016-06-06 05:09:57 PDT 
Comment on attachment 279672 [details]
Patch

Clearing flags on attachment: 279672

Committed r201709: <http://trac.webkit.org/changeset/201709>
Comment 7 WebKit Commit Bot 2016-06-06 05:10:02 PDT 
All reviewed patches have been landed.  Closing bug.