Bug 1828235 Comment 16 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Alex Jakobi from comment #14)
> should we reconsider using the initial fix Dan and I found, which was to put the [SetZoom](https://searchfox.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp#5995) call inside an if statement to prevent it being called when `aFlags` contains `PAN_INTO_VIEW_ONLY`, or should we keep working on resolving the issues described above?

If the suggestion in comment 15 doesn't avoid the issues you found, but this alternative fix does, then I think it makes sense to go with this alternative fix for now, as it seems pretty harmless.

However, we should file a follow-up bug to continue exploring the original fix, because I think that computing a minimum zoom which is higher than the current zoom can lead to other problems which fall outside of the `PAN_INTO_VIEW_ONLY` case. A couple of specific scenarios that come to mind are:

 1. A page that don't use `touch-action: none`, and where the input element is the full width of the viewport (or close to it). In such cases, we don't want to zoom in (since the input element would overflow the viewport horizontally), but the incorrect min-zoom will cause us to anyways.
 2. Double-tap zoom, which also uses ZoomToRect. The first double-tap zooms in, a second double-tap should zoom back out to the previous zoom level. An incorrect min-zoom will prevent us from zooming all the way out.
(In reply to Alex Jakobi from comment #14)
> should we reconsider using the initial fix Dan and I found, which was to put the [SetZoom](https://searchfox.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp#5995) call inside an if statement to prevent it being called when `aFlags` contains `PAN_INTO_VIEW_ONLY`, or should we keep working on resolving the issues described above?

If the suggestion in comment 15 doesn't avoid the issues you found, but this alternative fix does, then I think it makes sense to go with this alternative fix for now, as it seems pretty harmless.

However, we should file a follow-up bug to continue exploring the original fix, because I think that computing a minimum zoom which is higher than the current zoom can lead to other problems which fall outside of the `PAN_INTO_VIEW_ONLY` case. A couple of specific scenarios that come to mind are:

 1. A page that doesn't use `touch-action: none`, and where the input element is the full width of the viewport (or close to it). In such cases, we don't want to zoom in (since the input element would overflow the viewport horizontally), but the incorrect min-zoom will cause us to anyways.
 2. Double-tap zoom, which also uses ZoomToRect. The first double-tap zooms in, a second double-tap should zoom back out to the previous zoom level. An incorrect min-zoom will prevent us from zooming all the way out.
(In reply to Alex Jakobi from comment #14)
> should we reconsider using the initial fix Dan and I found, which was to put the [SetZoom](https://searchfox.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp#5995) call inside an if statement to prevent it being called when `aFlags` contains `PAN_INTO_VIEW_ONLY`, or should we keep working on resolving the issues described above?

If the suggestion in comment 15 doesn't avoid the issues you found, but this alternative fix does, then I think it makes sense to go with this alternative fix for now, as it seems pretty harmless.

However, we should file a follow-up bug to continue exploring the original fix, because I think that computing a minimum zoom which is higher than the current zoom can lead to other problems which fall outside of the `PAN_INTO_VIEW_ONLY` case. A couple of specific scenarios that come to mind are:

 1. A page that doesn't use `touch-action: none`, and where the input element is the full width of the viewport (or close to it). In such cases, we don't want to zoom in (since the input element would overflow the viewport horizontally), but the incorrect min-zoom will cause us to anyways.
 2. Double-tap zoom, which also uses ZoomToRect. The first double-tap zooms in, a second double-tap should zoom back out to the original zoom level. An incorrect min-zoom will prevent us from zooming all the way out.

Back to Bug 1828235 Comment 16