Bug 224224

Summary: WebKit is inconsistent about which side to place scrollbar on (with writing-mode & direction CSS properties)
Product: WebKit Reporter: Daniel Holbert <dholbert>
Component: Layout and RenderingAssignee: Cameron McCormack (:heycam) <heycam>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bfulgham, cdumez, cgarcia, changseok, emilio, esprehn+autocc, ews-watchlist, fred.wang, glenn, gustavo, gyuyoung.kim, heycam, kondapallykalyan, macpherson, menard, mifenton, pdr, simon.fraser, smoley, twilco.o, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar, WPTImpact
Version: Safari 14   
Hardware: Unspecified   
OS: Unspecified   
URL: https://bugzilla.mozilla.org/attachment.cgi?id=9213617
Bug Depends on: 224409, 224468    
Bug Blocks: 224357    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Squashed patch with dependencies for EWS
none
Squashed patch with dependencies for EWS
none
Squashed patch with dependencies for EWS
none
Squashed patch with dependencies for EWS
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Daniel Holbert 2021-04-05 23:08:10 PDT
STR:
(1) View https://bugzilla.mozilla.org/attachment.cgi?id=9213617
(2) Look at the scrollbars inside the two black boxes. (you may have to manually scroll them to make the scrollbars show up, if you've got overlay scrollbars.)

EXPECTED RESULTS:
The scrollbar should be on the same side of the two black boxes. (Both of these boxes have "right-to-left" content flows, via `direction:rtl` for the first one and via `vertical-rl` for the second one.)

In particular: I think the scrollbar should be on the **left side** for both boxes. (In both cases, the left side is the logical "end" side -- it's the inline-end edge of the the first box, and it's the block-end side for the second box.).

ACTUAL RESULTS:
Safari/WebKit places the scrollbar on the **right side** of the second box (the `vertical-rl` one).  This is the logical "start" side (specifically "block-start"), which is an odd place for a scrollbar.


I think this is the only case where Safari does this (where they place the scrollbar on a "start" side of a box).

Firefox gives EXPECTED RESULTS here.
Chrome matches Safari and gives ACTUAL RESULTS, but that's a Chrome bug, and it sounds like they're likely changing to match Firefox on this, per https://bugs.chromium.org/p/chromium/issues/detail?id=1195933#c6 . (That's the Chrome version of this issue.)

I tested this in Safari 14 on macOS Big Sur.
Comment 1 Daniel Holbert 2021-04-05 23:12:30 PDT
Note that there's a WPT test that inadvertently depends on this behavior (and fails in Firefox as a result), which is how I ran across cross-browser discrepancy:
https://wpt.fyi/results/css/css-flexbox/overflow-auto-005.html

I may end up fixing that WPT test over in https://bugzilla.mozilla.org/show_bug.cgi?id=1703074 , and/or it might get fixed as part of the bug report. Either way, it'll likely end up flipping its implicit expectation about scrollbar placement (which isn't really what it's intending to test anyway). (unless there's a case to be made that both behaviors are sensible, and if the text can be relaxed such that it allows both somehow)
Comment 2 Daniel Holbert 2021-04-05 23:12:54 PDT
(In reply to Daniel Holbert from comment #1)
> unless there's a case to be made that both
> behaviors are sensible, and if the text can be relaxed such that it allows
> both somehow)

Sorry, s/text/test/
Comment 3 Simon Fraser (smfr) 2021-04-06 09:43:48 PDT
Seems like a bug, yeah.
Comment 4 Smoley 2021-04-07 14:41:40 PDT
I can reproduce this on Safari 13.1.2 as well as TOT 14.2.
Comment 5 Radar WebKit Bug Importer 2021-04-07 14:41:49 PDT
<rdar://problem/76366184>
Comment 6 Cameron McCormack (:heycam) 2021-04-08 01:03:27 PDT
Created attachment 425482 [details]
Patch
Comment 7 Cameron McCormack (:heycam) 2021-04-08 18:45:47 PDT
Created attachment 425568 [details]
Patch
Comment 8 Cameron McCormack (:heycam) 2021-04-08 20:59:23 PDT
(In reply to Daniel Holbert from comment #1)
> Note that there's a WPT test that inadvertently depends on this behavior
> (and fails in Firefox as a result), which is how I ran across cross-browser
> discrepancy:
> https://wpt.fyi/results/css/css-flexbox/overflow-auto-005.html
> 
> I may end up fixing that WPT test over in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1703074 , and/or it might get
> fixed as part of the bug report. Either way, it'll likely end up flipping
> its implicit expectation about scrollbar placement (which isn't really what
> it's intending to test anyway). (unless there's a case to be made that both
> behaviors are sensible, and if the text can be relaxed such that it allows
> both somehow)

Hey Daniel, do you think css/css-writing-modes/sizing-orthog-vrl-in-htb-012.xht also needs fixing?  That's a case where the reference is using `writing-mode: vertical-rl` on the root, while the test itself has the default horizontal-tb.  But scrollbar position is not what's being tested.
Comment 9 Cameron McCormack (:heycam) 2021-04-08 21:01:10 PDT
css/cssom-view/cssom-getBoundingClientRect-vertical-rl.html too.
Comment 10 Cameron McCormack (:heycam) 2021-04-08 21:35:23 PDT
Created attachment 425584 [details]
Patch
Comment 11 Cameron McCormack (:heycam) 2021-04-08 21:57:56 PDT
r?dholbert https://github.com/web-platform-tests/wpt/pull/28425
Comment 12 Daniel Holbert 2021-04-09 08:32:34 PDT
Yeah, that seems fine (marked r+ in WPT repo).

It surprises me that Firefox places the scrollbars on the right side in those tests (I checked, and we do).  It seems the root scrollport is special for some reason; I'm not sure if that's intentional or not.

But yeah, in any case, scrollbar-side-placement isn't what those testcases are trying to test.
Comment 13 Daniel Holbert 2021-04-09 08:54:14 PDT
I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1704122 on the fact that Firefox seems to have special right-side-scrollbars-only behavior on the root element.
Comment 14 Cameron McCormack (:heycam) 2021-04-10 19:02:07 PDT
Created attachment 425694 [details]
Squashed patch with dependencies for EWS
Comment 15 Cameron McCormack (:heycam) 2021-04-12 21:41:26 PDT
Created attachment 425831 [details]
Squashed patch with dependencies for EWS
Comment 16 Cameron McCormack (:heycam) 2021-04-12 22:33:35 PDT
Created attachment 425835 [details]
Squashed patch with dependencies for EWS
Comment 17 Cameron McCormack (:heycam) 2021-04-12 23:21:34 PDT
Created attachment 425839 [details]
Squashed patch with dependencies for EWS
Comment 18 Cameron McCormack (:heycam) 2021-04-13 16:47:32 PDT
Created attachment 425931 [details]
Patch
Comment 19 Cameron McCormack (:heycam) 2021-04-13 17:25:33 PDT
Created attachment 425934 [details]
Patch
Comment 20 Cameron McCormack (:heycam) 2021-04-13 22:42:54 PDT
Created attachment 425953 [details]
Patch
Comment 21 Cameron McCormack (:heycam) 2021-04-15 02:01:19 PDT
Created attachment 426087 [details]
Patch
Comment 22 Cameron McCormack (:heycam) 2021-04-15 14:11:24 PDT
Created attachment 426137 [details]
Patch
Comment 23 Cameron McCormack (:heycam) 2021-04-15 18:08:12 PDT
Created attachment 426168 [details]
Patch
Comment 24 EWS 2021-04-16 16:55:24 PDT
Committed r276182 (236665@main): <https://commits.webkit.org/236665@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 426168 [details].
Comment 25 Tyler Wilcock 2021-04-16 17:26:55 PDT
Reopening to attach new patch.
Comment 26 Tyler Wilcock 2021-04-16 17:26:58 PDT
Created attachment 426296 [details]
Patch
Comment 27 EWS Watchlist 2021-04-16 17:27:46 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 28 Tyler Wilcock 2021-04-16 17:34:48 PDT
Sorry, not sure why `webkit-patch` uploaded to this bug.  Closing again.