Summary: | [css-grid] Different width of grid container between initial load and refresh | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Javier Fernandez <jfernandez> | ||||||||||||||||||||
Component: | Layout and Rendering | Assignee: | Javier Fernandez <jfernandez> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, darin, esprehn+autocc, glenn, hyatt, jfernandez, kondapallykalyan, rego, rniwa, simon.fraser, svillar | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
See Also: | https://bugs.chromium.org/p/chromium/issues/detail?id=628565 | ||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||
Bug Blocks: | 60731 | ||||||||||||||||||||||
Attachments: |
|
Description
Javier Fernandez
2016-10-17 00:18:06 PDT
Created attachment 291795 [details]
Patch
Comment on attachment 291795 [details] Patch Attachment 291795 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2302965 New failing tests: fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html fast/css-grid-layout/grid-track-sizing-with-percentages-and-orthogonal-flows.html Created attachment 291798 [details]
Archive of layout-test-results from ews103 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 291795 [details] Patch Attachment 291795 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2302969 New failing tests: fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html fast/css-grid-layout/grid-track-sizing-with-percentages-and-orthogonal-flows.html Created attachment 291799 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 291795 [details] Patch Attachment 291795 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2302970 New failing tests: fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html fast/css-grid-layout/grid-track-sizing-with-percentages-and-orthogonal-flows.html Created attachment 291801 [details]
Archive of layout-test-results from ews117 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 291795 [details] Patch Attachment 291795 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2302978 New failing tests: fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html fast/css-grid-layout/grid-track-sizing-with-percentages-and-orthogonal-flows.html Created attachment 291802 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 291805 [details]
Patch
Fixed layout tests.
Comment on attachment 291805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291805&action=review > Source/WebCore/ChangeLog:18 > + algorithm depend on these override sizes. Typo: s/depend/depends/ > Source/WebCore/ChangeLog:22 > + clear these override sizes and performs a layout of the affected grid Typo: s/performs/perform/ > Source/WebCore/rendering/RenderGrid.cpp:465 > + // We need to clear both own and containingBlock override sizes to > + // ensure we get the same result when grid's intrinsic size is > + // computed again in the updateLogicalWidth call bellow. We do this only for orthogonal items, shouldn't we say that on the comment. > Source/WebCore/rendering/RenderGrid.cpp:471 > + // We do cache orthogonal items during the placeItemsOnGrid call, which is > + // executed later. However, we are > + // only interested on running this logic when we are performing a > + // relayout, hence we have already cached > + // the orthogonal items. Mmm, you don't use cached orthogonal items here. Is this comment accurate? > LayoutTests/fast/css-grid-layout/repeating-layout-must-produce-the-same-results.html:19 > +.i1 { > + color: magenta; > + background: cyan; > +} Nit: Maybe you can use firstRowFirstColumn class which is more descriptive and already has a background color. And you just add here the color for the text. > LayoutTests/fast/css-grid-layout/repeating-layout-must-produce-the-same-results.html:20 > +.i2 { Ditto but using firstRowSecondColumn. > LayoutTests/fast/css-grid-layout/repeating-layout-must-produce-the-same-results.html:21 > + color: red; It's weird to use "red" if there's not a fail in a test. > LayoutTests/fast/css-grid-layout/repeating-layout-must-produce-the-same-results.html:30 > + document.body.offsetLeft; > + document.body.offsetLeft; Do you need to repeat twice this line? For future reference this has been already solved in Blink: https://codereview.chromium.org/2361373002/ Comment on attachment 291805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291805&action=review >> LayoutTests/fast/css-grid-layout/repeating-layout-must-produce-the-same-results.html:30 >> + document.body.offsetLeft; > > Do you need to repeat twice this line? Well, perhaps it's too much, but I wanted to be really sure that many layouts doesn't produce different results. I guess we can live with just one extra layout, but I guess we can keep as it is, so we performs the same test than Blink. Created attachment 291810 [details]
Patch
Applied suggsted changes.
Created attachment 291814 [details]
Patch
Properly forcing relayout in the test.
Created attachment 291829 [details]
Patch
Patch for landing.
Comment on attachment 291829 [details] Patch Clearing flags on attachment: 291829 Committed r207460: <http://trac.webkit.org/changeset/207460> All reviewed patches have been landed. Closing bug. |