1608506 - Avoid using mobile viewport sizing in zooming tests that run on desktop

Status: RESOLVED FIXED

(Core :: Panning and Zooming, task, P3)

Description

[Botond Ballo [:botond]]

The motivation for this is explained in bug 1556556 comment 35, but in a nutshell: we want zooming-related test coverage on desktop, but we don't want to use mobile viewport sizing on desktop, because it interacts badly with layout scrollbars, and we don't want to support that combination.

So, I'd like to go through our APZ tests that involve zooming and currently use mobile viewport sizing (i.e. an meta-viewport tag), and:

• if mobile viewport sizing is essential to the test (e.g. it's testing meta viewport behaviour), move the test to a test group that's restricted to mobile (and perhaps macOS, i.e. platforms that don't have layout scrollbars);
• otherwise, revise the test to cause zooming by some other means than a meta viewport tag.

Comment 1

[Botond Ballo [:botond]]

Note that we've already done work in this direction in https://phabricator.services.mozilla.com/D30989 (bug 1459260). I'm hoping to take that further here.

Comment 2

[Botond Ballo [:botond]]

Here's a preliminary analysis of APZ tests that use mobile viewport sizing:

• test_group_minimum_scale_size.html: These tests specifically test mobile viewport sizing, and so should be restricted to mobile / macOS.
• test_group_touchevents-3.html
  • helper_touch_action.html: Uses minimum-scale=1.0.
• test_group_touchevents-4.html
  • helper_bug1509575.html: Uses width=2000.
• test_group_zoom.html
  • helper_zoomed_pan.html: Uses minimum-scale=1.0.
  • helper_fixed_position_scroll_hittest.html: Uses initial-scale=2.0.
  • helper_basic_doubletap_zoom.html: Uses width=2100.
  • helper_visual_smooth_scroll.html: Uses minimum-scale=1.0 and initial-scale=2.0.
  • helper_scroll_into_view_bug1516056.html: Uses initial-scale=2.0.
  • helper_scroll_into_view_bug1562757.html: Uses initial-scale=2.0.
  • helper_fixed_pos_displayport.html: Uses minimum-scale=1.0 and initial-scale=8.0.
  • The remaining subtests just use width=device-width.
• test_group_zoom-2.html
  • helper_bug1280013.html: Uses width=2500.

My proposed revisions are as follows:

• Tests that just use width=device-width, initial-scale=1.0, and/or minimum-scale=1.0 should have equivalent behaviour on desktop with mobile viewport sizing disabled. (These things essentially disable aspects of default mobile viewport sizing behaviour on mobile.)
• Tests that use initial-scale other than 1 should be able to switch to using nsIDOMWindowUtils.setResolutionAndScaleTo() instead.
• Tests that use width to create a wide layout viewport need to be handled case-by-case.
  • helper_bug1509575.html and helper_bug1280013.html do this to get a layout viewport that's wider than the visual viewport at unit zoom. These tests can probably be reformulated to use zooming instead.
  • helper_basic_doubletap_zoom.html can probably be reformulated to just have a wide content size instead.

Comment 3

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #2)

• test_group_minimum_scale_size.html: These tests specifically test mobile viewport sizing, and so should be restricted to mobile / macOS.

Looks like this is already restricted to mobile only.

Comment 4

[Botond Ballo [:botond]]

I have most of these working locally, except for one:

(In reply to Botond Ballo [:botond] from comment #2)

• helper_basic_doubletap_zoom.html can probably be reformulated to just have a wide content size instead.

Double-tap zooming is gated on having a wide layout viewport width, and so cannot be triggered with mobile viewport sizing disabled.

For now, I think I'll move this subtest into a new test_group_double_tap_zoom test group which only runs on mobile.

At some point, we'll want to enable double-tap zooming for desktop as well (e.g. bug 674371). At that point, the gating condition linked above will need to be revised, and test_group_double_tap_zoom can probably enabled for desktop as well.

Comment 5

[Botond Ballo [:botond]]

Attachment: Bug 1608506 - Only force dom.meta-viewport.enabled=true for APZ subtests that actually need it. r=hiro

Comment 6

[Botond Ballo [:botond]]

Attachment: Bug 1608506 - Use nsIDOMWindowUtils.setResolutionAndScaleTo() rather than initial-scale to zoom in most APZ mochitests. r=hiro

The exception is the tests in test_group_minimum_scale_size.html, which are
specifically testing mobile viewport sizing behaviour.

Depends on D59586

Comment 7

[Botond Ballo [:botond]]

Attachment: Bug 1608506 - Avoid using mobile viewport sizing in test_group_touchevents-4.html. r=hiro

Depends on D59587

Comment 8

[Botond Ballo [:botond]]

Attachment: Bug 1608506 - Avoid using mobile viewport sizing in test_group_zoom-2.html. r=hiro

Note, in helper_bug1280013.html, since we are now zooming to 2x, the quantities
specified in CSS pixels need to be scaled down by 2x accordingly, to maintain
the relative positions of the elements and gestures on the screen.

Depends on D59588

Comment 9

[Botond Ballo [:botond]]

Attachment: Bug 1608506 - Split helper_basic_double_tap_zoom.html out into its own test group which only runs on mobile. r=hiro

This test needs mobile viewport sizing because we gate double tap zooming on that.

Depends on D59589

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec6194dd2e25
Only force dom.meta-viewport.enabled=true for APZ subtests that actually need it. r=hiro
https://hg.mozilla.org/integration/autoland/rev/20b367eea138
Use nsIDOMWindowUtils.setResolutionAndScaleTo() rather than initial-scale to zoom in most APZ mochitests. r=hiro
https://hg.mozilla.org/integration/autoland/rev/925d323581b2
Avoid using mobile viewport sizing in test_group_touchevents-4.html. r=hiro
https://hg.mozilla.org/integration/autoland/rev/10c4b29373fe
Avoid using mobile viewport sizing in test_group_zoom-2.html. r=hiro
https://hg.mozilla.org/integration/autoland/rev/492f2ffadfa2
Split helper_basic_double_tap_zoom.html out into its own test group which only runs on mobile. r=hiro

Comment 11

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/ec6194dd2e25
https://hg.mozilla.org/mozilla-central/rev/20b367eea138
https://hg.mozilla.org/mozilla-central/rev/925d323581b2
https://hg.mozilla.org/mozilla-central/rev/10c4b29373fe
https://hg.mozilla.org/mozilla-central/rev/492f2ffadfa2

Comment 12

[Hiroyuki Ikezoe (:hiro)]

(In reply to Botond Ballo [:botond] from comment #4)

I have most of these working locally, except for one:

(In reply to Botond Ballo [:botond] from comment #2)

• helper_basic_doubletap_zoom.html can probably be reformulated to just have a wide content size instead.

Double-tap zooming is gated on having a wide layout viewport width, and so cannot be triggered with mobile viewport sizing disabled.

For now, I think I'll move this subtest into a new test_group_double_tap_zoom test group which only runs on mobile.

At some point, we'll want to enable double-tap zooming for desktop as well (e.g. bug 674371). At that point, the gating condition linked above will need to be revised, and test_group_double_tap_zoom can probably enabled for desktop as well.

For posterity I am replying to this old comment since our mochitest.ini still points to this comment.

Today I tried to run test_group_double_tap_zoom-2.html on my Linux box but it doesn't work at all. That's because when we process the first touchend event we do set mSingleTapSent depending on whether the TAPGESTURE_UP was consumed or not. And in fact we do return nsEventStatus_eConsumeNoDefault, thus we never send TAPGESTURE_DOUBLE on desktop platform other than Mac.

It will be changed when we support double-tap-zoom on Windows/Linux.