1160756 - Reader View maximum text size insufficient

Status: RESOLVED FIXED

(Firefox for iOS :: Reader View, defect)

Description

[Boris Dušek]

Attachment: ReaderFirefoxSafariMaxFontSizeComparison.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/600.5.17 (KHTML, like Gecko) Version/8.0.5 Safari/600.5.17

Steps to reproduce:

1. Open a webpage having Reader Mode available (e.g. http://en.wikipedia.org/wiki/Firefox )
2. Launch Reader Mode
3. Tap the display settings button and choose maximum font size
4. Tap outsize of the display settings popover to look at the webpage


Actual results:

The size is not sufficient for people with need for big letters (low-vision people, includin also simply many older people).


Expected results:

The text size is sufficient - see attached comparison of maximum size in Safari's Reader (with big enough letters) and Firefox's Reader Mode.

Safari's text size is such that any bigger would probably mean words would not fit in screen width, so it is probably the maximum reasonable size (or very close to it).

Comment 1

[Boris Dušek]

From brief look, this would probably just require adding new cases to the ReaderModeFontSize enum, and adjust the Reader mode's CSS to include styles for these (i.e. after the maximum .font-size5, have also .font-size6, .font-size7 etc.).

Comment 2

[Boris Dušek]

Attachment: Pull request

Comment 3

[Brian Nicholson (:bnicholson)]

Darrin/Robin, what do you think of these new font sizes?

Comment 4

[Robin Andersen [:tecgirl]]

I think all we would need to do here is expand how many steps we can are utilizing. Currently we are only using 4 (total), while Safari has 11 (3 down, 8 up from default).

My suggestion: expand steps to 12: 4 down, 8 up.

Comment 5

[Wesley Johnston (:wesj)]

9 up would be one more than Safari. Wouldn't that make us better?

Comment 6

[Darrin Henein [:darrin]]

Removing ni.

Comment 7

[Robin Andersen [:tecgirl]]

:wesj actually, I added 1 extra step to the smaller size, so yeah, we're one-upping already.

Comment 8

[Brian Nicholson (:bnicholson)]

Comment on attachment 8601190 [details] [review]
Pull request

Looks good, but I think we want one more size as mentioned in comment 4.

Comment 9

[Boris Dušek]

Robin, I am not sure how to exactly implement this. In the pull request, I am using these sizes in pixels for main body text (let's focus on those for main body text as they are the most important, I will be able to adjust the other 2 types of sizes based on the answers to questions below):

size1: 14px
size2: 16px
size3: 18px
size4: 20px
size5: 22px
size6: 25px
size7: 28px
size8: 31px
size9: 35px
size10: 40px
size11: 45px
size12: 50px

The size1 to size5 were already there before the PR. I left the default to be size3. So now we have one default size (size3), 2 steps down (to size1), 9 steps up (to size12).

I was thinking adding one size smaller than size1 (e.g. size0), having size of 12px or 13px, and moving the default size to be size4 instead of size3. This would make us have the required number of steps up and down. It would make the default font size ~10% bigger though (from 18px to 20px).

So I was wondering if you are OK with this, or have some other suggestion on how to achieve the suggested number of sizes/steps. Thanks.

Comment 10

[Robin Andersen [:tecgirl]]

The default body text is spec'd to 16px (1em), so how does the below work? (body copy sizes only, we can also figure out what the h1 sizes are)

12px	0.750em	
13px	0.813em	
14px	0.875em	
15px	0.938em	
16px	1.000em	(baseline)
17px	1.063em	
18px	1.125em	
19px	1.188em	
20px	1.250em	
21px	1.313em	
22px	1.375em	
23px	1.438em	
24px	1.500em

Comment 11

[Robin Andersen [:tecgirl]]

Boris, where are we at with this?

Comment 12

[Boris Dušek]

Hi Robin, apologies for being unresponsive lately. I will finally get to a response during the next 2 days.

Comment 13

[Boris Dušek]

Hello Robin,

the sizes you proposed, that is 13 levels (1 default + 4 smaller + 8 larger) ranging 12px - 24px in body font size, are in my opinion insufficient w.r.t maximum size. I have no problem with the number of levels or the smallest size, but the biggest size is IMHO too small. You are proposing maximum body (.content) size of 24px, in the pull request I originally proposed 45px (i.e. roughly twice as bigger) as that is comparable to the maximum Reader body font sizes Safari uses (and which is beneficial to people who have some more serious visual impairment, which was my primary motivation for this, but I could imagine this might be useful even during some presentations of content to public).

I updated the pull request attached to this bug to have new code that:
- is rebased on latest master
- reflects the total number of 13 size levels you suggested (as opposed to 12 I originally proposed), including 4 steps down / 8 steps up from the default
- respects the 16px default body font size
- stipulates a 48px maximum body size (slightly bigger than Safari's maximum, and is roughly the threshold that with any bigger size, it would no longer be an exception that one word does not fit on the width of a screen of iPhone in portrait mode)
- stipulates geometric series (i.e. each next value is the same multiple of the previous one); specifically with size5 being 16px and size13 being 48px, I got that the size of each level should be ~14.7% more than the previous level. I think this "feels" more natural than increasing linearly (i.e. by a fixed number of pixels each level), but I could be wrong.

The results (after some adjustment for 2 smallest sizes, and one overriding of rounding in size12) for the body size are:

size1: 10px
size2: 11px
size3: 12px
size4: 14px
size5: 16px
size6: 18px
size7: 21px
size8: 24px
size9: 28px
size10: 32px
size11: 37px
size12: 42px
size13: 48px

I am attaching a screenshot of each size (source: https://en.wikipedia.org/wiki/Serena_Williams)

I am also attaching a Numbers table where I calculated the sizes (you can modify the "Anchors" table, i.e. which 2 levels have fixed values and which are those, and the "Body sizes" table recalculates). I also left there values for hypothetical levels beyond 13 (see note about iPad below) for an idea how the sizes could evolve, or to enable experimenting with smaller changes between each level (i.e. to anchor size5 @ 16px, and size17 @ 48px).

(both attachments will be in follow-up comments, as they can't be attached to this comment directly)

Could you please tell me what do you think about these body sizes? Would you like me to use different sizes?

I should also note that Safari makes the last 2 steps a big increase (i.e. while other steps increase by approx. 1-2 px, the last 2 steps both increase by 8px. But I find no reason why we would want to emulate that behavior as well.

One last thing: I think it would be proper to have even bigger maximum size on iPad (to use the larger screen estate), but I propose not to complicate things with it now and to leave it for a follow-up bug. I am just mentioning it in case you would like to somehow consider it already now.

I have modified the other 2 size types (1 for headings, 1 for domain/caption/credits), but only to the extent to bring them up to date with your recent changes (i.e. to make size6 of "h1" bigger than the new size5). I think that, as you suggested, it's better we work out the body sizes first and then specify the other 2.

Looking forward to your feedback, and once again sorry for the delay in my response.

Comment 14

[Boris Dušek]

Attachment: Reader View body sizes screenshots

Screenshot of Reader View's body size for each of the 13 sizes. (source page: https://en.wikipedia.org/wiki/Serena_Williams)

Comment 15

[Boris Dušek]

Attachment: Reader View body size calculation

Numbers table I used to calucate sizes of Reader View's body font, and that can be used to experiment with them. Modify entries in the "Anchors" table, and the sizes in "Body" table update.

Comment 16

[Robin Andersen [:tecgirl]]

I think the table you worked out is great. I did only intend for 12 steps, but see now how it became 13. The rest look good and scale nicely. I am working on the CSS for iPad right now, and would like to take a look at this on iPad. Do you have a branch I can test with?

Thank for you for your work on this!

Comment 17

[Boris Dušek]

(In reply to Robin Andersen [:tecgirl] from comment #16)
> Do you have a branch I can test with?

Yes, branch accessibility-bug1160756-readermode-bigger in my fork of firefox-ios on github (i.e. dusek/firefox-ios): https://github.com/dusek/firefox-ios/tree/accessibility-bug1160756-readermode-bigger

Also there is a pull request for exactly this branch: https://github.com/mozilla/firefox-ios/pull/419

Comment 18

[Boris Dušek]

Also on iPad, I think one way to approach it would be that we could simply expand the number of sizes (i.e. add size14, size15 up to size21 as suggested above) (simply add them to the ReaderModeFontSize enum and then define them in the CSS), and have the Swift code return different value for ReaderModeFontSize.isLargest (it could return Size13 on iPhone and Size21 on iPad).

Comment 19

[Richard Newman [:rnewman]]

Over to st3fan for landing. Robin, chime in if anything more is needed from you!

Comment 20

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I have merged this. If iPad specific changes are wanted then we can do that as part of bug 1173885