1460873 - Clicking on zoomed images does not display them centered and zoom indicator doesn't reflect the true level

Status: NEW

(Core :: Layout, defect, P3)

Description

[Oana Botisan, Desktop QA]

Attachment: zoom not centered (1).gif

[Affected versions]:
- Firefox 60.0;
- Firefox 60.0ESR;
- Firefox 61.0b3;
- Firefox 62.0a1 (2018-05-10) 

[Affected platforms]:
- Win10x64,
- macOS10.12, 
- Ubuntu 16.04x64.

[Steps to reproduce]:
1. Launch Firefox.
2. Navigate to the following link: http://ilab.engr.utk.edu/iLabDocs/Epilog/BMP%20sample%20files/lady3.bmp
3. Zoom in(ex: hold CTRL and keep pressing the + key).
4. Click on the image.

[Expected result]:
- The image should zoom out.

[Actual result]:
- The image is scrolled to the bottom-right corner-edge.
- The browser shows that the image is zoomed at 100% but it's still increased above that value.

[Regression range]:
- Using mozregression I've found this pushlog that might have caused the issue: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4f4487cc2d30d988742109868dcf21c4113f12f5&tochange=aabfe960ab59fea2e85896b1f8050786e16ab23b
- Last good: 20171207100053
- First bad: 20171208220141

[Additional notes]:
- See attached screen-cast.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

(In reply to Oana Botisan from comment #0)
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=4f4487cc2d30d988742109868dcf21c4113f12f5&tochange=aabf
> e960ab59fea2e85896b1f8050786e16ab23b

This doesn't match the last good/first bad dates you provided.

> - Last good: 20171207100053
> - First bad: 20171208220141

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Also, I don't think this is a real bug.

(In reply to Oana Botisan from comment #0)
> [Actual result]:
> - The image is scrolled to the bottom-right corner-edge.

Where you end up depends on what part of the image you click on in step 4 of your STR.

> - The browser shows that the image is zoomed at 100% but it's still
> increased above that value.

Yes, there's different kinds of zoom involved here. Doing ctrl++ is layout zoom but clicking on an image is a different kind of zoom, so when you click it resets the layout zoom to 100% and switches to the other kind of zoom (let's call it "image zoom"). You get image zoom even if you click on the image without doing layout zoom. Clicking on the image again undoes the image zoom and restores it to fit-to-visible-area.

Comment 3

[Oana Botisan, Desktop QA]

Sorry about the pushlog. I ran mozregression again and this is the real one:

- https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c35ad676551a69dba04514ee7d00e98a6332b367&tochange=b8174ee90b5820f2dba829e2f0c400013da8deb4

I still think it is, maybe not the part with 100%, but the part when you click on the image and it jumps it is.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Regression from bug 1149555 according to comment 3.

Comment 5

[Andrei Vaida [:avaida]]

Desktop QA has been highlighting this defect as a known issue for several release cycles now, following regression testing. Daniel, it would really help us if you set a priority on this bug — it will give us clarity on whether this issue is still worth mentioning in future test reports.

Comment 6

[Daniel Holbert [:dholbert]]

I think this is likely not a high priority bug. Taking a look to briefly reassess the issue while I'm here, though.

I'm actually having a hard time seeing what the regression here was.

I see what-looks-to-me like the "bad" behavior in Nightly 2017-12-06 (a day before comment 3's regression range) as well as in much older builds from e.g. 2014-01-02. (I see the "jump" based on where you click, and I see the "image zoom" taking over from full-page-zoom at that point [using kats' terminology] to still zoom in but in a different way; and I'm not immediately seeing any difference related to the regression range.)

Oana, can you clarify what the behavior-change was in the regression range from comment 3 here?

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (large image that can be used for testing)

Here's an image that can be used for testing purposes -- the image linked in comment 0 isn't available anymore, but I think any image that's larger than the browser viewport (as this one should be for reasonably-sized monitors) should exercise the relevant behavior/features

Comment 8

[Daniel Holbert [:dholbert]]

to be clear, I definitely agree that:
(A) the click-triggered swap between types-of-zoom ("full page zoom on unzoomed image" to "image shown at full-scale") does feel clunky and unintuitive
(B) the fact that the click jumps to a part of the image not-in-the-viewport when you click at the bottom-right feels pretty unexpected.

Part (A) is not something that can just be fixed, since it just falls out of the fact that we have two different methods of zoom here. I think we would need a fair amount of redesign how the image viewing UI works to improve things on that front, probably with some input from UX folks etc, which would be a bit of a project and is probably not high-priority to get resourced any time soon, I'd bet.

Part (B) does feel like something that could be fixed. I think we're interpreting the click as if it were a click in the viewport when the whole image is being displayed (scaled down to fit the viewport), and we're jumping to show you the corresponding part of the image when we swap it to display at its full resolution. So clicks in the bottom right of the viewport take you to the bottom right of the image. We probably should be taking the current zoom level into account there so that we can figure out what part of the image you actually clicked on. I suspect the relevant code for that geometry is here:
https://searchfox.org/mozilla-central/rev/a26e972e97fbec860400d80df625193f4c88d64e/dom/html/ImageDocument.cpp#468-479

Having said that, though, both of these issues (A)/(B) are things that I'm seeing in builds from before the regression range that's been identified here, so I'm not sure if there's yet-another-issue separate from A/B that I'm not seeing, or if they started manifesting slightly differently around that range in a way that I'm not seeing, or what.

Comment 9

[Oana Botisan, Desktop QA]

You are right. The regression range for this issue is not correct. I can reproduce the bug on very old builds (2011-01-02). I am not even sure that this bug has a regression range.

But just to clarify things the issue here is that at the first click the image jumps and only at the second click it zooms out. The expected behaviour here should be that the image is zoomed out at the first click. However if this can't be fixed, at least the jumping part would still be an improvement, because it's really strange that it jumps as well.

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to Oana Botisan, Desktop QA from comment #9)

You are right. The regression range for this issue is not correct. I can reproduce the bug on very old builds (2011-01-02). I am not even sure that this bug has a regression range.

Thanks for confirming that

But just to clarify things the issue here is that at the first click the image jumps and only at the second click it zooms out. The expected behaviour here should be that the image is zoomed out at the first click.

So, I totally agree that this is unintuitive, but it is in fact "correct", as kats sort-of explained at the end of comment 2. Chromium and WebKit have this same behavior as well (though it's a bit extra/confusing for them since they retain the full-page-zoom as a supplemental zoom factor when you click to toggle here).

As kats noted in comment 2, there are two different kinds of zoom here:
(A) Images that are too large will automatically get scaled-down to fit the viewport at first load, which we can call "image scale-down zoom".
(B) As with any page, you can use Ctrl +/- to zoom in/out using "full page zoom".

Here's what happens:

• When you first load the image, we have "image scale down zoom" applied.
• Then when you do Ctrl +, you are applying a bit of full-page-zoom to that scaled-down view.
• When you click the image, it turns off both forms of zoom and you see the image at full resolution (no zoom is applied at all). This feels like you're zooming in, but really you're just snapping to see the image at its full resolution.
• When you click the image again, it reenables "image scale down zoom" and you go back to the initial rendering.

Chrome and WebKit have the same behavior as us, except that they don't cancel out the full-page-zoom in the 3rd step, so you see the image at full resolution with some full-page-zoom applied to it which ends up feeling more-broken and less intuitive (in my opinion) since you have to do two different things to get back to a regular view of the full image. (Those things being (1) clicking and (2) doing Ctrl -, to undo each type of zoom that you have applied)

So, bottom line: this isn't the most usable interface, and it might be worth pointing a UX designer at how we might redesign this at some point; but for now at least, the behavior here is as-intended and at-least-as-understandable as the competition.

However if this can't be fixed, at least the jumping part would still be an improvement, because it's really strange that it jumps as well.

Yup. I'm going to spin off a separate bug to focus specifically on that, as a dependency of this one.

Comment 11

[Daniel Holbert [:dholbert]]

--> I spun off bug 1903880 for the "jump" (described in e.g. comment 8 part B)