Make scrollbars visible on mac screenshots

[davidsgrogan]

Tests that exercise layout's response to scrollbars can bogusly pass on mac because mac doesn't show scrollbars even with overflow:scroll.

I discovered this when looking at a test @mrego added with a Blink bugfix. The bug was when table cells have scrollbars, their %height children were sized incorrectly. Chrome 66 is buggy and Chrome 68 is correct. Safari STP 55 and below is buggy. But the test passes in Chrome 66 and Safari on macOS because no scrollbars appear that would affect layout.

I proposed a quick fix of styling the scrollbars in this test with ::-webkit-scrollbar* and friends. But perhaps the test runner should always apply something more rigorous on mac and not leave it to individual tests? Blink does something similar in its testing.

https://jsfiddle.net/dgrogan/me8esewq/4/ shows actual and expected renderings with both default and custom scrollbars. If you run it on chrome 68 linux, chrome 68 mac, chrome 66 linux, chrome 66 mac, Safari, etc you'll see what the issue is.

/cc @mstensho @gsnedders

[gsnedders]

cc/ @youennf

[foolip]

@davidsgrogan do you know what is that causes scrollbars to be visible in content_shell? In Safari, is the problem that the scrollbars are visible to a user, but not captured in the screenshot? Or that they aren't visible at all?

[gsnedders]

@foolip They're not visible to the user; scrollbars on macOS by default disappear except when scrolling.

[foolip]

Ah, right. @youennf, any way to change the default?

[gsnedders]

@foolip defaults write -g AppleShowScrollBars -string Always will set it to always be visible; can also set it on a per-app basis (drop the -g, specify the bundle identifier instead)

Of course, that moves us away from the configuration that most users run with, which might also be problematic (e.g., if we want to test disappearing scrollbar interaction with things like percentage widths and the vw/vh units, which is known to not be interoperable).

[mrego]

@davidsgrogan do you know what is that causes scrollbars to be visible in content_shell?

There's a custom "theme" special for form controls and scrollbars: https://cs.chromium.org/chromium/src/content/shell/test_runner/mock_web_theme_engine.cc?type=cs&sq=package:chromium&l=28

[gsnedders]

@mrego that's all #if !defined(OS_MACOSX), so none of that is used on macOS.

[davidsgrogan]

Some digging around content_shell reveals:

https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_test_controller.cc?l=528&rcl=db5a7efe37ac6f283f8dfc411f5f0fe57cc082a9

mock_scrollbars_enabled = true;

Eventually checked by https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/scroll/scrollbar_theme.cc?l=384&rcl=6156ddee9c553723c0bdbfb2bae14865cecca53f

which causes ScrollbarThemeMock to be used instead of ScrollbarThemeMac.

The ScrollbarThemeMock header says "Scrollbar theme used in image snapshots, to eliminate appearance differences between platforms."

@jeremyroman or @progers , could you confirm and possibly loop in someone who was part of tackling this issue (mac doesn't show scrollbars by default) in content_shell?

[mrego]

There are similar things on WebKit: https://trac.webkit.org/browser/trunk/Source/WebCore/platform/mock/ScrollbarThemeMock.cpp

This is a screenshot of how scrollbars are visible when running tests in WebKit on Mac platform, while they're not visible if you open the same page in Safari.

[mrego]

As a somehow related note I've found this property: https://drafts.csswg.org/css-overflow-4/#scollbar-gutter-property That comes out of this discussion: https://github.com/w3c/csswg-drafts/issues/92

This should allow to know what to do with scrollbars sizes on the test, but until we don't have this I don't know what we should do regarding overly scrollbars in Mac.

[frivoal]

Right. The property that rego pointed to are meant precisely for that kind of thing. Since there's both user demand for this, and they would solve the problem for tests, I'd suggest trying to implement that.

[frivoal]

Until now, nothing defined what the scrollbars should look like or how much space, if any, they should take. This css property gives some control over that

[progers]

@davidsgrogan, sometimes we want to test behavior on mac as it would be in the default configuration (with scrollbar hiding), and sometimes we want to force scrollbars on. The testrunner supports both configurations. I don't quite understand your question; do you want to change something in this area?

[davidsgrogan]

@progers Sorry, I was asking if you could confirm that the code I linked to in https://github.com/w3c/web-platform-tests/issues/10972#issuecomment-389360424 is indeed how layout tests force scrollbars on when we want them to.

I did not realize we have the option to run layout tests with or without scrollbars forced on, but it makes total sense. How is that controlled? Do individual layout tests specify in the test itself?

[foolip]

Hmm, if this needs to be controlled dynamically, I wonder if it could be a WebDriver API. @kereliuk, WDYT?

[progers]

@davidsgrogan, I think these are the two flags: // controls the platform-specific scrollbars vs the mock ones if (window.testRunner) testRunner.setUseMockTheme(false);

// controls the overlay behavior where scrollbars do not take up space. if (window.internals) internals.runtimeFlags.overlayScrollbarsEnabled = true;

As far as I l know, these are set individually in tests. I think the default is to use the mock theme and not use overlay scrollbars.

[davidsgrogan]

@foolip I interpret priority:backlog to indicate this won't be resolved soon. In the meantime, I suppose the guidance for us test authors is to include custom css to make the scrollbars visible when necessary?

[foolip]

@davidsgrogan, that was me triaging all infra issues via https://foolip.github.io/ecosystem-infra-rotation/.

Right now it looks like there doesn't exist any standardized and implemented CSS that would solve the problem. Would support for scrollbar-gutter: always solve the problem, or would the scrollbar itself still not be visible on macOS?

Concretely, as long as the tests are passing in content_shell, I would probably leave them failing in Chrome on wpt.fyi and point to this issue, or an issue to implement something in Chrome that'd solve the problem.

[davidsgrogan]

Would support for scrollbar-gutter: always solve the problem, or would the scrollbar itself still not be visible on macOS?

Yep, scrollbar-gutter: always would solve the problem.

as long as the tests are passing in content_shell, I would probably leave them failing in Chrome on wpt.fyi

The issue is that tests are passing where they should fail, i.e. Safari and Chrome<=67 on mac. https://wpt.fyi/results/css/css-tables/height-distribution/percentage-sizing-of-table-cell-children-002.html should show everything as failing, but Safari shows as passing.

I suppose instead of using the -webkit-scroll properties in these tests, we could supplement the ref matching with script that prints the used value. It seems a little better, but clearly doesn't solve the problem for others who aren't aware of it.

[foolip]

I guess that does present a bit of a problem, people are less likely to prioritize tests that are incorrectly passing than incorrectly failing, so if we use only scrollbar-gutter then it wouldn't be implement just in order to fail more tests, probably.

Would the same CSS in all tests involving scrollbars work? In that case perhaps we could have a shared CSS file like your scrollbars.css in https://chromium-review.googlesource.com/c/chromium/src/+/1054770 plus scrollbar-gutter: always would do the trick, if there is a subset of those two things that would do exactly the same thing.

We've been trying to remove prefixed things from wpt when they would cause things to pass without the unprefixed things being supported, but this is the other way around. @jgraham may have opinions on whether we should be keen on prefixed things to make things fail correctly.

[davidsgrogan]

Would the same CSS in all tests involving scrollbars work?

That would for sure work for layout tests that depend on scrollbars to change the content box size. But are you suggesting to automatically inject that css into pages that wpt detects has scrollbars? That might obviate tests whose whole point is to check overlay scrollbars, but I'm at the frontier of my knowledge here and don't really know.

But if you're suggesting test authors manually use a common scrollbars.css when they know it's needed on mac, then I think that would work, though then a test author would have to be somewhat aware of this issue in the first place.

if there is a subset of those two things that would do exactly the same thing.

I think you're suggesting to add scrollbar-gutter properties that do the same thing as the -webkit-scroll properties, so that engines that don't implement -webkit-scroll would pick up this behavior when they implement scrollbar-gutter?

[davidsgrogan]

I ended up submitting https://chromium-review.googlesource.com/c/chromium/src/+/1054770 as-is with its ad hoc solution to this problem. I'm not sure what the long-term solution should be. Ideally we would run all the tests in multiple configurations -- default scrollbars, overlay scrollbars, mock scrollbars -- because tests should pass in each, but that's obviously too expensive. So perhaps we want to add something to webdriver (https://github.com/web-platform-tests/wpt/issues/10972#issuecomment-389640286) that does something like what @progers showed in https://github.com/web-platform-tests/wpt/issues/10972#issuecomment-389652565. Then at least test authors who know about this issue can control how scrollbars are rendered for their tests.

@kereliuk, WDYT?