64926 - Page is not rescrolled after text zoom (font size changed) (page position) (increase text size)

Status: NEW

(Core :: Layout, defect, P4)

Description

[Konstantin Riabitsev]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.16 i686; en-US; 0.7) Gecko/20010105
BuildID:    2001010517

Not sure whether this is fixable, knowing that Mozilla depends on X for fonts,
but nevertheless:
If you're in the middle of a lengthy page and you change your font size by doing
View->Text size, the fonts change, but your page doesn't acommodate for the
newer font sizes, showing either the part of the page you've already read
through (if you made fonts bigger), or the part you're not into yet (if you made
them smaller). This is really annoying, since you have to scroll looking for
where you left off.

Reproducible: Always
Steps to Reproduce:
Scroll to the middle of a long page.
View->Font Size->; Change setting

Actual Results:  You're elsewhere in the page.

Expected Results:  You should see the same fragment of the text.

Mozilla should re-calculate font sizes and then re-scroll to show you the same
portion of text which you were viewing before you changed the size of your fonts.

Comment 1

[Konstantin Riabitsev]

An afterthought: This can be done by grabbing a portion of the visible text or
anchoring the visible part of the page somehow, then changing the font size and
looking for the grabbed text/anchor, which should be then scrolled into view.
Thus no font calculations are necessary.

Comment 2

[timeless]

yes this is important.

Comment 3

[Peter "jag" Annema]

Perhaps this could reuse the code currently used to remember the current
position on a page when going back/forward?

Comment 4

[timeless]

good idea.

Comment 5

[Keyser Sose]

Triaging the Most doomed (which Happens to Ben right now :)

Comment 6

[Keyser Sose]

*** Bug 72099 has been marked as a duplicate of this bug. ***

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

Not XPApps/FE. 

Comment 8

[Marc Attinasi]

Does anybody know where the current scroll-to logic is? I am not familiar with it...

Comment 9

[Eric Hanchrow]

This seems like a duplicate of Bug 136564

Comment 10

[Marcus Campbell]

*** Bug 136564 has been marked as a duplicate of this bug. ***

Comment 11

[Boris 'pi' Piwinger]

*** Bug 167368 has been marked as a duplicate of this bug. ***

Comment 12

[Boris Zbarsky [:bzbarsky]]

*** Bug 180024 has been marked as a duplicate of this bug. ***

Comment 13

[Hixie (not reading bugmail)]

->layout again

Comment 14

[Andrew Hagen]

*** Bug 167007 has been marked as a duplicate of this bug. ***

Comment 15

[Andrew Hagen]

This is not an enhancement. It is a defect in our existing implementation. This
bug makes text zoom problematic to use on long pages. Normal severity.

Changing summary to make this easier to find. The feature in question is no
longer called font size, but now it's called "text zoom."

This bug should be very easy to fix. First, this bug has the following symptoms.

You're scrolling down in a page, let's say Slashdot's front page, that has
several screenfuls worth of information. You decide to text zoom. It doesn't
matter whether you increase or decrease the size of the text. When the text size
is changed, the vertical scrollbar moves. As a result, the position on the page
that you were just reading is not on the screen anymore. 

All we have to do to fix this bug is to not move the scrollbar.

A potential fix would be this. Before changing the text size, we should figure
out how far the user has scrolled down, figured as a percentage of the page size
or length taken as a whole. Then the text size should be changed. Then, after
that, we should move the scroll bar up to or down to the same percentage of the
page as was figured before.

Comment 16

[brian]

Another potential solution would involve determining what element is displayed
in the top left corner of the screen and then repositioning so that the same
element is there after the zoom.

I'm not familliar with the code, but intuitively it seems like this could use a
lot of the same stuff that's used for the 'view selection source' feature on the
context menu.

I'll be very happy to see this bug fixed :)

Comment 17

[Jesse Ruderman]

See also bug 139832 (for resizing windows), which suggests an approximate method
of getting the right scroll position that involves keeping the scroll percent
rather than the number of pixels.

Comment 18

[Boris Zbarsky [:bzbarsky]]

*** Bug 171297 has been marked as a duplicate of this bug. ***

Comment 19

[Andrew Hagen]

I was just thinking while text zooming at the osnews.com forum, it would be
great to do this not just for vertical scroll position, but also for horizontal
scroll position. The same algorithm should work for both.

Comment 20

[Oliver Klee]

*** Bug 229108 has been marked as a duplicate of this bug. ***

Comment 21

[Eyal Rozenberg]

Another bug which should be really easy to fix, and would contribute a lot to
the browsing experience.

BTW, for whoever fixes this: I suggest that the center of the viewport will
remain fixed, that is to say if the middle character in the line closest to the
middle of the viewport is the X%'th character on the page, this should remain
the case (give or take 1 line) after the font resize as well.

Comment 22

[alanjstr]

I would say the top of viewport should be fixed, not the middle.  If I can't
read the top line and I increase the size, I wouldn't want it vanishing on me. 
It's also the way IE does it.  

Comment 23

[Eyal Rozenberg]

But,alanjstr, think that, usually, before the actual text on a page there are
headers, titles, etc. - plus you usually start reading a few lines, then give up
and zoom in. That's why I think the center is better. I also think zooming into
the center has a generally-less-disorienting effect.

Comment 24

[Eyal Rozenberg]

Technically, it seems there are 2 ways to go about fixing this bug: either the
Javascript code which sets the zoom (
/xpfe/communicator/resources/content/*iewZoom* ) will also change the
nsIMarkupDocumentViewer's idea of what's its position in the document so as to
fit  some zoom repositioning policy (nsIMarkupDocumentViewer doesn't seem to
have a 'position' attribute, it must be somewhere else and I can't figure out
for now exactly where that is), or, alternatively, the _way_
nsIMarkupDocumentViewer (or whatever entity exposes it to the zoom setting code)
thinks about where it's at in a document can change, e.g. if the browser window
shows text lines 150-170 in font 12pt then instead of thinking "I am at
150*(12+spacing) pt's from the start" it will think "I am at a position in which
the first displayed line is 150" or "I am in a position in which the middle
displayed line is 160".

Comment 25

[Jan Gundtofte-Bruun]

I fear comment #16 wouldn't, as work lots of sites use some form of menu along
the left border that is way narrower and shorter than the actual page body --
using this for an anchor may lock the view to the wrong part of the page.

Therefore, I too vote to use the percentage of the whole page (as measured by
the scrollbars or however, so that the topmost line stays more ore less in
place). Most user interfaces are anchored in top-left, and especially for text
displays this would (imho) make the most sense.

And yes, it's only natural to handle the horizontal position as well.

Comment 26

[Ash Argent-Katwala]

I can think of one class of cases where this is well defined. Where you have
just gone to a location with a named anchor, the right place to rebind the
viewport to would be to rerun the logic for showing the named anchor.

The rest of the cases are plainly quite tricky, but it may be useful to handle
this set separately anyway.

Comment 27

[Jesse Ruderman]

*** Bug 243879 has been marked as a duplicate of this bug. ***

Comment 28

[R.K.Aa.]

*** Bug 251726 has been marked as a duplicate of this bug. ***

Comment 29

[Pascal Sartoretti]

> *** Bug 251726 has been marked as a duplicate of this bug. ***

I created the dupe, sorry. The summary should include "increase text size" (or
decrease), as it is the name of the offending command.

By the way, I don't understand why this bug has not been corrected since 3
years, it is SOOOOOO annoying yet seems trivial to correct. Maybe I should quit
whining and start learning Mozilla development :-)

I have seen the lengthy discussion about what to implement (top of viewport,
center of viewport, percentage).... who cares? Any solution would be better than
the status quo.

Comment 30

[Andrew Hagen]

Please don't just say "Me, too."

As it turns out, this problem appears simple on the surface, but is not as
simple underneath.

This bug is ultimately dependent on bug 103279. That is where a lot of the
action is going to be. 

Comment 31

[Brian Meidell]

I suggest it should work like this:

When using keys to zoom text, the first line stays fixed.

When using ctrl-mousewheel and the mouse is over the client area, the line under
the mouse pointer is fixed.

Comment 32

[José Jeria]

*** Bug 254702 has been marked as a duplicate of this bug. ***

Comment 33

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

*** Bug 272919 has been marked as a duplicate of this bug. ***

Comment 34

[José Jeria]

*** Bug 274383 has been marked as a duplicate of this bug. ***

Comment 35

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

*** Bug 280460 has been marked as a duplicate of this bug. ***

Comment 36

[Felix Miata]

This bug can be seriously annoying for Chatzilla users. Normally, each channel
gets its own tab, and the focus in that tab is kept at the bottom - the
scrollbar button stays stuck to the bottom until one purposely moves it
elsewhere. Now if in the browser I need to temporarily change my default font
size smaller for some reason, there is no negative impact on CZ. However, when I
change the default size back up to normal, most (or all, depending on how much
content is in particular channels) of the CZ channel windows have scrolled up
off the bottom, and I have to move each back manually before I see any new
channel activity - the channel tailing is stopped until I scroll back to the bottom.

Comment 37

[Samuel Sieb]

The chatzilla issue is somewhat different than normal page viewing.  With
chatzilla you want the bottom of the page anchored, which is definitely what you
don't want for normal browsing.

Comment 38

[Felix Miata]

Does that mean CZ needs its own bug?

Comment 39

[Anne (:annevk)]

*** Bug 208756 has been marked as a duplicate of this bug. ***

Comment 40

[Frank Wein [:mcsmurf]]

*** Bug 304823 has been marked as a duplicate of this bug. ***

Comment 41

[Klaudiusz Gawlik]

Helo
Anybody know about work progress with this issue? Now I have FF 1.5b on MSWin
and this bug still exist...

Comment 42

[mrkiwi]

*** Bug 309922 has been marked as a duplicate of this bug. ***

Comment 43

[Adam Guthrie]

*** Bug 312801 has been marked as a duplicate of this bug. ***

Comment 44

[lsof]

Almost five years old!

Comment 45

[Luca M.]

try it here: http://www.badboy.com.au/

Comment 46

[Adam Guthrie]

*** Bug 321266 has been marked as a duplicate of this bug. ***

Comment 47

[Hideo Saito]

Attachment: patch

roc, could you review this patch?

I think that the zoomed scroll position is able to calculate using the rate of the scroll position and the content size. This approach improves this problem since the position is in near although it shifts from an original position.

Calculated scroll position is a center of the scrollbar in order to make a gap small, if scrollbars do not touch to the left/right side or top/bottom side.

If scrollbars touch to left/right side or top/bottom side, scrollbars keep on the side.

Comment 48

[Chris Conover]

Hi,

First off, I would like to thank Hideo Saito for the work submitted as part of the last post, there has now been five years of discussion so I am very pleased that this is being addressed.

I do think that the algorithm could be improved a little however.  To give the user intuitive zoom semantics, the zooming should be based on current cursor position.  This is consistent with graphics applications and games.  I am not familiar with the mozilla codebase, but the general approach is as follows:

Store state before zoom is executed:
	get current position of cursor within view
	calculate ratio of that position in the current view,
		- for example, it may be line 100 units from a total view of 400 units or 0.25
	calculate absolute position in document from cursor position for example, 500 units into a document of 1000 units
	(for sake of reference, this would mean that the view was displaying the view from 400-799 in the document)

Calculate new position after zoom:
	calculate new position by starting with absolute position, and calculate new start and end boundary based on the relative position of the cursor
		- for example, suppose zoomed view is now 200 units, then new relative position would be 0.25 * 200 = 50 (truncated if need be)
	calculate start of new view by subtracting new offset from original absolution position
		- in the example above, this would be 500 - 50 = 450, so the new view would span 450 - 649 (given a new view size of 200 units)

The final zoom would result in the change from a view spanning 400-799 to 450-649, and would zoom around the point that is 0.25 into the view.

-c

Comment 49

[Joshua Goldberg]

(In reply to comment #48)
> To give the
> user intuitive zoom semantics, the zooming should be based on current cursor
> position.  This is consistent with graphics applications and games.

When reading a long web page with page-down, the cursor tracks the bottom of the viewport, even though the user didn't explicitly put it there, or even read that far.  (This is a difference from graphics apps and games.)  Keeping the *top-left corner* of the current view fixed would make it more clearly visible how the view changed and help people stay oriented.

Comment 50

[Chris Conover]

As a response to comment 50, the fact that the cursor is at the bottom right of the view appears to be more of an implementation detail since it has no visible impact on the user experience (from testing on this bug report).  I also feel that while it may be simpler to make it zoom from the top of the page (despite the cursor / focus being at the bottom of the viewport) it is less intuitive, has less precedent, and is not as flexible.  It removes the user from the decision making process and negates the point of fixing this bug in the first place.  If the final result is that the user will have to rescroll the text after zooming to get to the point of interest that was scrolled off the bottom of the page since the top was fixed, then we won't have solved much.  The one work around for the user to scroll the relevant text to the top of the page and then zoom, which is just as inconvenient.  It should also be noted that since the user will often be moving the cursor and zooming with the mouse (a single input device), they will associate cursor position and zoom behavior quite quickly.

At the very least I think it would be prudent to do usability testing since this application gets such widespread usage.

regards,

-c

Comment 51

[Hideo Saito]

Comment on attachment 207624 [details] [diff] [review]
patch

bzbarsky, could you review this patch or can I get your advice?

My approach is that the rate of the scroll position against the content size is not chanded. The zooming is done suitably for my test. Should the zooming be based on current cursor position like as comment #48?

Comment 52

[brian]

I don't think cursor position is relevant here. We need to provide a view that allows the user to maintain their orientation within the page; keeping the top-left corner fixed will provide this. Content will always flow in and out of the right and bottom of the window, so it will be easy to know where you are and locate something that flows out of view on zoom.

As an aside, I tried figuring out how cursor position would affect things by turning caret browsing on (F7), but its easy for the caret to be off-screen, and we definitely do not want to zoom there. Caret browsing provides the most explicit cursor I know of, and it isn't what we want at all.

Let's get this reviewed and checked in so that page zoom can FINALLY be useable on long pages!

Comment 53

[Daniel]

Well, top-left corner is not correct solution. Zooming should always be made based on the middle-center position, e.g. absolute center of viewing port.

That's how zooming works everywhere - starting from Acrobat Reader, through Word, up to Photoshop. Changing this will certainly confuse user and is not recommended.

Comment 54

[Felix Miata]

Keeping 0,0 unchanged on zoom is the only way that makes sense and won't disorient me.

Comment 55

[Joshua Goldberg]

(In reply to comment #50)
> It removes the user from the decision making process

That's just it.  A user scrolling down a long page isn't involved in a
decision to put the focus to the lower corner.

I spent a little time looking at Acrobat right now, which seems to
zoom from the center (although only approximately; i.e., you don't
neccessarily get to where you started if you zoom in and out
repeatedly.)  It was okay once I figured out what was going on.  I
think I still might prefer top-left, but user-testing would of course be
ideal.  (Perhaps simply timing people to find their place again on a
text-heavy page after a zoom-in/out.)

PS. If it'll still be a long time before this is fixed, perhaps it's worth considering "smooth"-zooming?  

Comment 56

[Hideo Saito]

Attachment: patch

I added following changes in order to limit the effect of scaling scroll position only to the zooming text.

     ClearStyleDataAndReflow();
+    mPrevTextZoom = mTextZoom;
   }

Comment 57

[Boris Zbarsky [:bzbarsky]]

I really think that if we're going to mess with this we should fix it in the context of bug 43114, not on its own.  In any case, I will not be able to get to this review for a long time (probably several weeks at least).

Comment 58

[Boris Zbarsky [:bzbarsky]]

So to make it clear, I would recommend finding a different reviewer.

Comment 59

[Hideo Saito]

Comment on attachment 207899 [details] [diff] [review]
patch

David, could you review this patch? If this patch is unuseful and this problem should not be fixed now, I also need to wait for that time to come.

Comment 60

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Doing this using only math is likely to make things worse in almost as many cases as it improves (especially since the things near the top of the viewport, which are often what the user is focusing on, would now be likely to move offscreen instead of moving lower within the viewport).

I'd prefer an approach more like the one described in bug 16806 comment 39, something that's currently covered by bug 43114.

Comment 61

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 207899 [details] [diff] [review]
patch

My inclination (and I think Boris's is the same) is that we don't want to take this approach, both because:
 * we don't want something specific to just text zoom
 * this is the wrong algorithm for fixing the problem

Comment 63

[nathaniel]

Same problem exists for full page zoom.

Comment 65

[Doug Hockin]

There's a variant of this bug related to opening links in a new tab. If the link points to an anchor in the new page, then when you select that tab it initially displays the correct location of the anchor in the default font size, but then it immediately resizes the font to match the one you've set in the link source page
(this is a relatively new feature I _really_ like). But when it auto-resizes you loose the anchor off the top or bottom of the page. Will this variant be fixed by the fix for this problem, or should I submit a different problem report?

Comment 66

[Roman Bednarek]

It was reported: 
https://bugzilla.mozilla.org/show_bug.cgi?id=474770

Comment 67

[Jeffrey Hurwit]

(In reply to comment #21)
> BTW, for whoever fixes this: I suggest that the center of the viewport will
> remain fixed, that is to say if the middle character in the line closest to the
> middle of the viewport is the X%'th character on the page, this should remain
> the case (give or take 1 line) after the font resize as well.

This is the way that Adobe Acrobat does it. It makes the most sense when zooming up, because you really do not know which portion of the page that the user is viewing will be pushed out of the viewport. Take the center as your anchor and, on average, the user will not need to reposition the page very much to bring the desired bit back into view.

Comment 68

[Jeffrey Hurwit]

For what it's worth...

The vote count for this bug report stands at 30. Additionally, 21 bug reports were filed that have been marked as duplicates of this one. If those were counted as votes, the total would be 51. That's a lot of Me Too's. However, we cannot all be developers; even those of us who have a little coding experience (like me) may not be able to work at this level of complexity.

Yes, I get it that this issue is complicated by other scrolling-related problems. But it is an issue that negates much of the usability of the zoom feature. Can fixing it not be given priority over adding nifty new features, such as adding the activity indicator to the list of customizable toolbar elements?

By the way, someone posted a workaround to one of the Mozilla wishlist newsgroups: turn on caret browsing (F7), place the cursor on a line of text in your current area of focus, adjust the zoom, depress the left or right arrow key-- the page will scroll to bring the cursor back into the viewport (usually at the top or bottom of it), turn off caret browsing. It's not great, but is usable, and better than the current situation.

Comment 69

[Jeffrey Hurwit]

I found this very interesting link in the last comment of the discussion on bug 43114:

http://blogs.msdn.com/ie/archive/2009/05/18/making-the-web-bigger-part-1-improvements-to-zoom.aspx

I haven't tried IE8 yet; if the M$ development team managed to solve the problem in anything near the style they claim, they have at least as much bragging right as they show in their article.

Comment 70

[Vassily Gavrilyak]

I tried IE8, works properly

Comment 72

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 20 duplicates and 39 votes.
:dholbert, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 73

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.