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

Issue 759193004: Test: Don't add scrollbar width in case of fixed width grid container

Created:
6 years ago by Sunil Ratnu
Modified:
5 years, 9 months ago
CC:
blink-reviews, jfernandez, Manuel Rego, svillar
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Test: Don't add scrollbar width in case of fixed width grid container While working on https://codereview.chromium.org/535913002/, we saw that scrollbar width does not get added to width of fixed with grid container (the same way as in case of fixed width grid items). Its better to have the test for this as well. Hence, added the test. BUG=395788

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -0 lines) Patch
A LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width.html View 1 chunk +28 lines, -0 lines 3 comments Download
A LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width-expected.txt View 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (1 generated)
Sunil Ratnu
Hi Julien, Please review this. Thanks! Sunil
6 years ago (2014-11-26 05:39:00 UTC) #2
Sunil Ratnu
On 2014/11/26 05:39:00, Sunil Ratnu wrote: > Hi Julien, > > Please review this. > ...
6 years ago (2014-11-27 13:28:43 UTC) #3
Sunil Ratnu
On 2014/11/27 13:28:43, Sunil Ratnu wrote: > On 2014/11/26 05:39:00, Sunil Ratnu wrote: > > ...
6 years ago (2014-12-02 16:52:57 UTC) #4
Julien - ping for review
https://codereview.chromium.org/759193004/diff/1/LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width.html File LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width.html (right): https://codereview.chromium.org/759193004/diff/1/LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width.html#newcode16 LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width.html:16: <div class='grid' data-expected-width='100'> I don't know if this tests ...
6 years ago (2014-12-02 18:57:50 UTC) #5
Sunil Ratnu
Hi Julien, Sorry for the delay in my response as I was busy with a ...
5 years, 9 months ago (2015-03-16 11:50:17 UTC) #6
Julien - ping for review
5 years, 9 months ago (2015-03-16 20:49:56 UTC) #7
https://codereview.chromium.org/759193004/diff/1/LayoutTests/fast/css-grid-la...
File
LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width.html
(right):

https://codereview.chromium.org/759193004/diff/1/LayoutTests/fast/css-grid-la...
LayoutTests/fast/css-grid-layout/fixed-width-grid-container-should-not-include-scroll-bar-width.html:16:
<div class='grid' data-expected-width='100'>
On 2015/03/16 11:50:17, Sunil Ratnu wrote:
> Since, we do not don't report scroll bar width to offsetWidth, so we can not
> check using data-expected-width as pointed by you. I could not understand the
> last part of of your above comment.
> 
> Since we can not use data-expected-width, does there exist any other way by
> which we can check the issue this test is trying to check?

I don't think check-layout.js has what you want. We would need some layout
property that is impacted by the scrollbar presence like Element.clientWidth.
That would be the best option.

You will have to account for overlay scrollbars that don't count take any space
on some platforms too.

Powered by Google App Engine
This is Rietveld 408576698