847872 - Only reflow zoom on double tap

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Readability, defect)

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

moving this to tracking+ because we don't plan to uplift this work. It will ship in the release it is completed on trunk for (most likely 23).

Comment 2

[Scott Johnson (:jwir3)]

Attachment: b847872

Here's my take on the double-tap to reflow patch. Note that this patch makes some string changes, which UX will likely want to review.

Also, I've enabled reflow-on-zoom when pinch-zooming OUT, since a user could double-tap to zoom in, and then pinch to zoom out, causing a really strange disconnect in wrapped text when they were expecting to have smaller text that doesn't wrap. Brad, let me know if you want me to disable this, too, but for now, I thought it best if I leave it in. Pinch-to-reflow-text is disabled for pinch-zooming in.

Finally, this doesn't address the issue of double-tapping on an image, but we should probably look into that in another bug (which I think is already filed...)

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 745365 [details] [diff] [review]
b847872

Review of attachment 745365 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ -95,5 @@
>  <!ENTITY pref_plugins_enabled "Enabled">
>  <!ENTITY pref_plugins_tap_to_play "Tap to play">
>  <!ENTITY pref_plugins_disabled "Disabled">
>  <!ENTITY pref_text_size "Text size">
> -<!ENTITY pref_reflow_on_zoom2 "Pinch to reflow text">

you need to change the string id (so probably reflow_on_zoom3)

Comment 4

[Kent Fitzsimmons]

The problem with enabling text reflow only on double tap is that this will zoom to a fixed percentage. Many sites use different sizes of base fonts, and the primary users of zoom are people with reduced vision: older people and people with vision disabilities. 

For those people, it is essential to be able to pinch zoom to the desired font size which can be read, and then have the text reflow to fit the screen so that it is not necessary to scroll right and left on each line. 

Even if double tap was set to zoom to a fixed font size, this would not be ideal for people with reduced vision because they may need different font sizes at different times, depending on the condition of their eyes. I can speak from experience as one of these people.

Further, a cursory search around the web shows many threads of people complaining that text reflow on pinch zoom is not working properly in the default Samsung Galaxy S3 and S4 browsers, so there is clearly discontent with the lack of this feature. 

If you have good eyesight, it is easy to consider this feature as unnecessary or lower priority, but for people with poor vision this is a make or break feature requirement. Currently the only phones with stock browsers which I have found that do pinch zoom text reflow correctly are HTC, and for that reason those are the phones I have to purchase. 

Please don't limit Firefox to users with good vision by disabling continuous pinch to zoom with text reflow!

Comment 5

[Scott Johnson (:jwir3)]

Hi Kent:

(In reply to Kent Fitzsimmons from comment #4)
> The problem with enabling text reflow only on double tap is that this will
> zoom to a fixed percentage. Many sites use different sizes of base fonts,
> and the primary users of zoom are people with reduced vision: older people
> and people with vision disabilities. 

Somewhat contrary to this statement, reflow-on-zoom isn't an accessibility feature. What that means is that this feature isn't designed specifically to make the web easier to use for those with disabilities. It's a feature that's designed to make the web easier to read, on devices with smaller screens, for everyone. This point might seem arbitrary and irrelevant, but our decisions with respect to this feature are based on what's good for the majority of individuals likely to be using it. 

I'm not saying those with special difficulties reading text don't need a feature designed to assist them, I'm simply stating that isn't the goal of _this_ feature. If you find yourself having difficulties, it might be worth filing a bug under the Accessibility component, along with any proposals you might have to fix the issue, so that we can track it separately from this one.
  
> Even if double tap was set to zoom to a fixed font size, this would not be
> ideal for people with reduced vision because they may need different font
> sizes at different times, depending on the condition of their eyes. I can
> speak from experience as one of these people.

It isn't set to zoom to a fixed font size. Each individual user can control what font size they want to zoom to by using the font.size.inflation.minTwips preference. (Note that it's currently using this preference for double-tap-zoom, but I'm going to change it to its own preference, since this seems it will be less confusing in the long term).

Comment 6

[Scott Johnson (:jwir3)]

Made changes requested by blassey and pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=abde7892c6d5

Comment 7

[Scott Johnson (:jwir3)]

(In reply to Scott Johnson (:jwir3) from comment #6)
> Made changes requested by blassey and pushed to try:
> https://tbpl.mozilla.org/?tree=Try&rev=abde7892c6d5

Oops.. pushed the wrong patch. Here's the actual try server run:
https://tbpl.mozilla.org/?tree=Try&rev=aa441d6799dc

Comment 8

[Scott Johnson (:jwir3)]

Better, green try server run:
https://tbpl.mozilla.org/?tree=Try&rev=761ec855b4dc

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed0e9d2f23a0

Comment 9

[Scott Johnson (:jwir3)]

Backed out for bustage on robocop tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd7d0d2c53e1

Comment 10

[Scott Johnson (:jwir3)]

Re-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/385c05262943

(Accidentally pushed the wrong version of the patch last time).

Comment 11

[Scott Johnson (:jwir3)]

*sigh* 
Backed out again, due to robocop test failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36618922eb84

Comment 12

[Kent Fitzsimmons]

(In reply to Scott Johnson (:jwir3) from comment #5)
> Each individual user can control
> what font size they want to zoom to by using the
> font.size.inflation.minTwips preference. 

If it's not a very easy to find and modify preference, it won't be useful. 

How about looking at the other use case: zooming in without reflowing the text. We should be asking: how often that is what users would want? 

I would submit that having to scroll right and left for each line is almost never what any user wants, therefore reflowing should be the default and if users for some unusual reason want to have to scroll around a zoomed-in but unaltered page, they should have to specify that behavior.

Comment 13

[Erin Lancaster [:elan]]

Here are videos of the latest reflow behavior...

Comment 14

[Erin Lancaster [:elan]]

	
http://people.mozilla.com/~elancaster/ReflowFeedback/

Comment 15

[Scott Johnson (:jwir3)]

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3843112fdca3

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3843112fdca3

Comment 17

[Mathieu Pellerin]

This patch should be backed off until more work is done. double tapping now fails to do anything, zooming in doesn't reflow while zooming out will end up reflowing text.

Before, double taps would zoom viewport to element width under tap. that was useful,has there been a discussion over losing that behaviour in favour of this bug?

Comment 18

[Aaron Train [:aaronmt]]

(In reply to Mathieu Pellerin from comment #17)
> This patch should be backed off until more work is done. double tapping now
> fails to do anything, zooming in doesn't reflow while zooming out will end
> up reflowing text.

It is on Nightly which are builds for testing purposes. Thanks for testing. Please file bugs as you find them.

Comment 19

[Scott Johnson (:jwir3)]

We should probably consider uplifting this to Fx23, since right now, Firefox 23 has a suboptimal version of Reflow-on-zoom (the pinch-to-reflow-text version)

Comment 20

[Scott Johnson (:jwir3)]

(In reply to Scott Johnson (:jwir3) from comment #19)
> We should probably consider uplifting this to Fx23, since right now, Firefox
> 23 has a suboptimal version of Reflow-on-zoom (the pinch-to-reflow-text
> version)

Re-noming with correct flags. :)

Comment 21

[Lukas Blakk [:lsblakk] use ?needinfo]

Will consider a low risk uplift nomination but since the existing pinch-to-reflow is not a new feature and this is an enhancement there is no need to track this issue for a particular release.

Comment 22

[Teodora Vermesan (:TeoVermesan)]

Test Case added in moztrap:
https://moztrap.mozilla.org/manage/case/8225/