475997 - make zoomToPage use viewport width

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Comment 1

[(dormant account)]

Attachment: This zoom setting works much nicer for me

Comment 2

[(dormant account)]

Note http://hg.mozilla.org/mobile-browser/rev/d9deda3eb847 is not this patch. That commit message was supposed to refer to bug 468869

Comment 3

[(dormant account)]

Attachment: finalized patch

I think this zooming method is much nicer in terms of usability compared to what's currently in Fennec. This eliminates horizontal scrolling that had to be done on every single page before(including firstrun)

Comment 4

[(dormant account)]

Attachment: make use of viewportVisibleRect

Comment 5

[(dormant account)]

I'd like to clarify that this patch is based on the assumption that the canvas width should not equal the device screen width. I find this easier to think about when surfing on a portrait screen which requires lots of panning sideways.

Comment 6

[(dormant account)]

Comment on attachment 360790 [details] [diff] [review]
make use of viewportVisibleRect

I think Gavin objects to this patch because in his opinion the canvas should shrink down to the page size.

Personally I think the canvas should always be bigger than the screen in x&y directions for performance reasons. I claim that webpages should always zoom relative to the visible area.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

My understanding is that we both agree that ideally, the canvas would be relative to the screen size. We don't necessarily agree that the multiplier should be 1 in the width case (i.e. canvas the same width as the window), but that's somewhat orthogonal to this bug.

That patch trades off the need to scroll to view the entire page if the window is smaller than the viewport with the appearance of a white bar along the page edge if you do. I think it would result in a more visibly broken state by default, so I don't think we should take it as a stopgap measure.

Comment 8

[(dormant account)]

(In reply to comment #7)
> That patch trades off the need to scroll to view the entire page if the window
> is smaller than the viewport with the appearance of a white bar along the page
> edge if you do. I think it would result in a more visibly broken state by
> default, so I don't think we should take it as a stopgap measure.

Just to clarify that is not a new bug and will occur when manually zooming. I personally think that for now it's better to have empty space outside of the viewport then suffect the indignity of a clipped webpage :)

Gavin, how hard is it fix that bug in the widgetstack?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #8)
> Just to clarify that is not a new bug and will occur when manually zooming.

When manually zooming *out* from the default zoom, which occurs much less frequently, and doesn't really have a valid use case.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 360790 [details] [diff] [review]
make use of viewportVisibleRect

I don't think we should take something like this until bug 479753 is fixed (and depending on how we fix it, this might not be necessary).

Comment 11

[Matt Brubeck (:mbrubeck)]

These patches no longer apply; the functionality changes have been implemented since this bug was opened.