Fix zoom behavior on mobile

[cowuake]

Change preliminarily discussed in #859. The change fixes an erratic behavior when zooming in with a mobile web browser. The terminal height was changed accordingly to a change in the visual viewport, but without considering the scale factor. This led, e.g., to a terminal area half the expected one for a 2X zoom level. The fix does not alter the resizing behavior at constant scale factor for the appearing and disappearing of a virtual keyboard.

[coveralls]

coverage: 82.704% (-0.03%) from 82.729% when pulling e94c9c3e11045b9708ee24758276c2988e158668 on cowuake:devel into df3506af2665440dc3f3d6bda9b47d2131ceb483 on jcubic:devel.

[cowuake]

@jcubic, I had to revert the commit that implemented the refactoring you asked for in #859. The reason is that comparing the heights already multiplied by the scale and then rounded breaks again the behavior with the virtual keyboard - I tested the change on Android 13 with Firefox/Chrome/Edge.

[jcubic]

How it breaks if it's exact same code:

var height = window.visualViewport.height;
var scale = window.visualViewport.scale;
height = Math.round(height * scale);

and this is the same:

var scale = window.visualViewport.scale;
var height = Math.round(window.visualViewport.height * scale);

You just do this in two steps instead of one.

[jcubic]

Ok it seems you added this line for no reason:

var newHeight = Math.round(window.visualViewport.height * newScale);

this code was on in first commit.

[cowuake]

How it breaks if it's exact same code:

var height = window.visualViewport.height;
var scale = window.visualViewport.scale;
height = Math.round(height * scale);

and this is the same:

var scale = window.visualViewport.scale;
var height = Math.round(window.visualViewport.height * scale);

You just do this in two steps instead of one.

OK, so you wanted only those lines changed, i.e.:

var scale = window.visualViewport.scale;
var height = Math.round(window.visualViewport.height * scale);
callback(height);
window.visualViewport.addEventListener('resize', function() {
    var newHeight = window.visualViewport.height;
    var newScale = window.visualViewport.scale;
    if (height !== newHeight) {
        height = Math.round(newHeight * newScale);
        callback(height);
    }
});

This way we are comparing a scaled height with a non-scaled height (unless I'm missing something!!) with height !== newHeight, and that's the reason why I also changed the code below - the error was not reassigning the value of newHeight to height before callback invocation, hence the problematic behavior! If you're satisfied with this solution I will update the PR accordingly: I'd just move the newScale initialization/assignment down, since when we don't have to change the height we don't have to read the scaling factor either. It would become:

var scale = window.visualViewport.scale;
var height = Math.round(window.visualViewport.height * scale);
callback(height);
window.visualViewport.addEventListener('resize', function() {
    var newHeight = window.visualViewport.height;
    if (height !== newHeight) {
        var newScale = window.visualViewport.scale;
        height = Math.round(newHeight * newScale);
        callback(height);
    }
});

Storing the scaling factors in the scale and newScale variables could be avoided tout-court, but I'll follow your advice.

[jcubic]

Yea, you're right, but your code also have the same bug you compare scaled with non-scaled. It is triggering every time.

[cowuake]

Yea, you're right, but your code also have the same bug you compare scaled with non-scaled. It is triggering every time.

Updated the PR. Now it should all be OK. Tested again on Android 13. I changed more than those three initial lines but this way we only compare scaled heights. I fixed the issue that we had with my first (erroneous) refactoring.