Closed Bug 1664714 Opened 4 years ago Closed 4 years ago

turn on desktop zooming scrollbars for not webrender and not android

Categories

(Core :: Panning and Zooming, task)

task

Tracking

()

RESOLVED INVALID

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(1 obsolete file)

The desktop zooming scrollbars code is getting into better shape now and we are going to need actual testing of it very soon, unfortunately turning them on is blocked on bug 1663865.

We also don't need to turn them on for android since scrollbars are already working well enough there with pinch zooming and I don't want to cause any regressions there for now, we can enable them on android later once all the dust settles.

The desktop zooming scrollbars code is in better shape now. I'd like to enable it to get some wider testing. Unfortunately we can't do it for webrender because one of the patches we need to make tests green for desktop zooming scrollbars (bug 1663562) tickles an unrelated webrender bug (bug 1663865). So this patcho only enables the desktop zooming scrollbars for non-webrender. There are a number of complications in doing this. I will explain in detail so that I remember later.

In order for reftests to pass with desktop zooming scrollbars we need to use overlay scrollbars with a handful of reftests (one of the patches in bug 1663537). Adding that pref to reftests for the first time tickles a pre-existing font inflation issue (bug 1663562). The fix for that font inflation issue exposes an existing webrender bug (bug 1663865).

So in order to enable desktop zooming scrollbars only with non-webrender and still get green tests every where we need to incorporate the patch for that font inflation issue (bug 1663562) but only enable it for non-webrender. Then we need to skip the reftests that we annotate with the overlay scrollbars pref when running with webrender because otherwise we will get intermittent asserts (the font inflation issue) and one of the tests fails with the combination of desktop zooming scrollbars disabled and overlay scrollbars (because the desktop zooming code fixes that test when run with overlay scrollbars, but it passes without overlay scrollbars).

So this patch incorporates the patch for bug 1663562 but it's only active when wr is disabled.

We also incorporates the reftest overlay scrollbars patch from bug 1663537.

And then we skip those reftests with webrender.

Further, I changed gfxPlatform::UseDesktopZoomingScrollbars() so that it is a constant value after the first call rather than worry about it changing because webrender can get disabled while running.

I really don't want to have to do this, but we need wider testing of this code. Once the webrender bug (bug 1663865) is fixed we can back this out and fix everything the proper way.

Actually, I thought of a different way to do this: we can enable the scrollbars with webrender too if we create a pref and set it for mochitests in dom/base/test only, and the pref will disable the font inflation reflow patch.

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

Actually, I thought of a different way to do this: we can enable the scrollbars with webrender too if we create a pref and set it for mochitests in dom/base/test only, and the pref will disable the font inflation reflow patch.

This happened in https://phabricator.services.mozilla.com/D90181

Bug 1663865 got fixed so we don't need to jump through hoops anymore.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
Attachment #9175439 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: