New issue
Advanced search Search tips

Issue 904438 link

Starred by 2 users
Status: Fixed
Owner:
r...@igalia.com
Closed: Nov 15
Cc:
r...@igalia.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

[css-grid] Grid container with direction RTL wrongly considers scrollbar in right side

Reported by john.woo...@googlemail.com, Nov 12 
UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36

Steps to reproduce the problem:
1. Create a div with position absolute, direction rtl, display grid and overflow auto
2. Fill it with some contents so that it overflows
3. 

What is the expected behavior?
The scroll bar appears on the RHS and does not move the contents of the div to the right

What went wrong?
The scrollbar appears and shifts the contents of the div to the right

TESTCASE:

https://codepen.io/johnjwood/pen/GwNOjZ

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 70.0.3538.102  Channel: stable
OS Version: 10.0
Flash Version: 

I've tested it on Firefox and Edge - where it works fine

TESTCASE:

https://codepen.io/johnjwood/pen/GwNOjZ

Comment 1 by dtapuska@google.com, Nov 12

Components: Blink>Layout

Comment 2 by swarnasree.mukkala@chromium.org, Nov 13

Labels: Needs-Triage-M70

Comment 3 by woxxom@gmail.com, Nov 13 

Observed since at least r358816.

Comment 4 by mstensho@chromium.org, Nov 13

Cc: r...@igalia.com
Status: Available (was: Unconfirmed)

Comment 5 by r...@chromium.org, Nov 13

Components: -Blink>Layout Blink>Layout>Grid
Summary: [css-grid] Position absolute display grid direction rtl div does not overflow properly (was: Position Absolute display grid direction rtl div does not overflow properly)
Mmmm, I cannot reproduce the issue (at least on Linux).
Could you provide a screenshot/screencast just in case I'm not getting it properly?

Comment 6 by mstensho@chromium.org, Nov 13 

Taken with 72.0.3602.2 (Official Build) dev (64-bit) (Linux)
screenshot.png
385 KB View Download

Comment 7 by woxxom@gmail.com, Nov 13

chrome.png
11.3 KB View Download
FF.png
12.5 KB View Download

Comment 8 by r...@chromium.org, Nov 13 

Ok, I got it now I was looking to the wrong thing. Thanks.

Comment 9 by r...@chromium.org, Nov 13

Summary: [css-grid] Grid container with direction RTL wrongly considers scrollbar in right side (was: [css-grid] Position absolute display grid direction rtl div does not overflow properly)
BTW, this happens without position absolute too:
  <!DOCTYPE html>
  <div style="display: inline-grid; overflow-y: scroll; direction: rtl; border: solid thick; background: red;">
    <div style="background: lime;">PASS</div>
  </div>

Comment 10 by r...@chromium.org, Nov 13

Owner: r...@igalia.com
Status: Started (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/edf3fb623b35778a57380a0006124612b3e2bec5

commit edf3fb623b35778a57380a0006124612b3e2bec5
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Wed Nov 14 22:56:57 2018

[css-grid] Consider scrollbars in PopulateGridPositionsForDirection()

We never care about scrollbars
in LayoutGrid::PopulateGridPositionsForDirection(),
that's fine if the scrollbars are at the end
(e.g. on the right in horizontal writing mode and LTR direction)
but it causes problems when they're at the beginning
(e.g. on the left in horizontal writing mode and RTL direction).

The patch modifies the method so it takes into account scrollbar size
in order to compute the position of the columns/rows
depending on the direction and the writing mode.

BUG= 904438 
TEST=external/wpt/css/css-grid/grid-model/grid-container-scrollbar-001.html
TEST=external/wpt/css/css-grid/grid-model/grid-container-scrollbar-vertical-lr-001.html
TEST=external/wpt/css/css-grid/grid-model/grid-container-scrollbar-vertical-rl-001.html

Change-Id: I9c8307638647e449f6975cacf99f89e29d4fa655
Reviewed-on: https://chromium-review.googlesource.com/c/1334370
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Commit-Queue: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#608154}
[add] https://crrev.com/edf3fb623b35778a57380a0006124612b3e2bec5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-scrollbar-001-ref.html
[add] https://crrev.com/edf3fb623b35778a57380a0006124612b3e2bec5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-scrollbar-001.html
[add] https://crrev.com/edf3fb623b35778a57380a0006124612b3e2bec5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-scrollbar-vertical-lr-001-ref.html
[add] https://crrev.com/edf3fb623b35778a57380a0006124612b3e2bec5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-scrollbar-vertical-lr-001.html
[add] https://crrev.com/edf3fb623b35778a57380a0006124612b3e2bec5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-scrollbar-vertical-rl-001-ref.html
[add] https://crrev.com/edf3fb623b35778a57380a0006124612b3e2bec5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-model/grid-container-scrollbar-vertical-rl-001.html
[modify] https://crrev.com/edf3fb623b35778a57380a0006124612b3e2bec5/third_party/blink/renderer/core/layout/layout_grid.cc

Comment 12 by r...@igalia.com, Nov 15

Status: Fixed (was: Started)
►

Sign in to add a comment