1251280 - Top of the image is missing after zooming in at the maximum zoom level

Status: VERIFIED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Loic]

Reported here: http://forums.mozillazine.org/viewtopic.php?p=14521289#p14521289

STR:
1) Open http://i.imgur.com/Kwlo6.jpg
2) Hold Ctrl and start rotating the mouse wheel until you reach the maximum zoom level

Result:
Only a small part of the zoomed image is visible and top is missing even if the vertical scrollbar is on top.

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

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yeah, ok.  Zooming seems to keep the current zoom state but change whether the image overflows...

Note to self: need to use either the parent of changeset 93f722efcdec or some changeset after bug 1251796 is fixed to test fixes, since between those zooming is just broken on standalone images.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Another note to self: we get into CheckOverflowing and all that, but the basic problem is that we're in the "shrinkToFit" state (in which in theory we never have vertical overflow) but we're not actually shrinking to fit once the zoom happens.  Need to think a bit about what to do about this.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: When zooming or out on a resized full-page image, we may need to update our vertical-overflow class even if we don't change anything else

Sorry to tag you for this, Kyle, but you reviewed bug 812899....

I tried to come up with a way to test this, and haven't figured it out yet.  I'll try to think about it a bit more, I guess, if we really care.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> When zooming or out 

"When zooming in or out".  Fixed locally.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d8a648e89b6

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6d8a648e89b6

Comment 7

[Boris Zbarsky [:bzbarsky]]

Loic, thank you for filing this and hunting down the regression range!

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Loic, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 9

[Krzysztof]

Not really jet.

Comment 10

[Loic]

(In reply to Ritu Kothari (:ritu) from comment #8)
> Loic, could you please verify this issue is fixed as expected on a latest
> Nightly build? Thanks!

I tested on Win 7 with the latest Nightly, zoom in/out are fixed (both (non-)e10s).
Built from https://hg.mozilla.org/mozilla-central/rev/05c087337043dd8e71cc27bdb5b9d55fd00aaa26

Comment 11

[Krzysztof]

Could you test this bug with the following js function? (using Scratchpad)

https://gist.github.com/TyDraniu/2aeb9fa8e5b87ed6d068

Comment 12

[Loic]

(In reply to Krzysztof from comment #11)
> Could you test this bug with the following js function? (using Scratchpad)
> 
> https://gist.github.com/TyDraniu/2aeb9fa8e5b87ed6d068

It's still broken.

Comment 13

[Boris Zbarsky [:bzbarsky]]

I'm not quite sure what the function in comment 11 has to do with this bug...

Comment 14

[Krzysztof]

@Boris, do you mean that I should file another bug? Or there is no bug at all? 

Running this function 3-5 times on img gives the same effect as this bug: top of the image is missing and you can't go to the top using vertical scrollbar.

Comment 15

[Boris Zbarsky [:bzbarsky]]

> @Boris, do you mean that I should file another bug? 

At the very least, yes.  This bug was about the built-in image zoom UI not working properly.

> Or there is no bug at all? 

Possibly, yes.  It depends on what sorts of things we want to support here.

> top of the image is missing and you can't go to the top using vertical scrollbar.

Right, because you're manually messing with the state ImageDocument set up and ImageDocument doesn't know you did that.  The same thing would happen if you set "margin-top: -1000px" on the <img>, for example...

Comment 16

[Krzysztof]

Thanks for the explanation. 

The strange thing is that it was working few versions earlier. I was using this function quite often as a bookmarklet to zoom in/out images.

Comment 17

[Boris Zbarsky [:bzbarsky]]

It was working because we hadn't fixed bug 812899.

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Loic from comment #10)
> (In reply to Ritu Kothari (:ritu) from comment #8)
> > Loic, could you please verify this issue is fixed as expected on a latest
> > Nightly build? Thanks!
> 
> I tested on Win 7 with the latest Nightly, zoom in/out are fixed (both
> (non-)e10s).
> Built from
> https://hg.mozilla.org/mozilla-central/rev/
> 05c087337043dd8e71cc27bdb5b9d55fd00aaa26

Thank you for the verification.