Closed Bug 847872 Opened 7 years ago Closed 7 years ago

Only reflow zoom on double tap

Categories

(Firefox for Android Graveyard :: Readability, defect)

All
Android
defect
Not set

Tracking

(firefox23-)

RESOLVED FIXED
Firefox 24
Tracking Status
firefox23 - ---

People

(Reporter: blassey, Assigned: jwir3)

References

Details

(Whiteboard: [readability])

Attachments

(1 file)

No description provided.
OS: Mac OS X → Android
Hardware: x86 → All
tracking-fennec: ? → 22+
Whiteboard: [readability]
Assignee: bugmail.mozilla → sjohnson
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).
tracking-fennec: 22+ → +
Attached patch b847872Splinter Review
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...)
Attachment #745365 - Flags: review?(blassey.bugs)
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)
Attachment #745365 - Flags: review?(blassey.bugs) → review+
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!
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).
Made changes requested by blassey and pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=abde7892c6d5
(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
Better, green try server run:
https://tbpl.mozilla.org/?tree=Try&rev=761ec855b4dc

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed0e9d2f23a0
Target Milestone: --- → Firefox 23
Backed out for bustage on robocop tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd7d0d2c53e1
Target Milestone: Firefox 23 → ---
Re-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/385c05262943

(Accidentally pushed the wrong version of the patch last time).
Target Milestone: --- → Firefox 23
*sigh* 
Backed out again, due to robocop test failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36618922eb84
Target Milestone: Firefox 23 → ---
(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.
Here are videos of the latest reflow behavior...
Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3843112fdca3
Target Milestone: --- → Firefox 24
Flags: in-moztrap?(fennec)
https://hg.mozilla.org/mozilla-central/rev/3843112fdca3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Depends on: 873721
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?
Depends on: 873969
(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.
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)
(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. :)
tracking-fennec: + → ?
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.
Test Case added in moztrap:
https://moztrap.mozilla.org/manage/case/8225/
Flags: in-moztrap?(fennec) → in-moztrap+
tracking-fennec: ? → ---
Product: Firefox for Android → Fennec Graveyard
You need to log in before you can comment on or make changes to this bug.