1158294 - Increase Reader Views Default Type Size

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Michael Maslaney [:mmaslaney] ([e-mail])]

Let's increase the size of Reader's default type size by one increment, this would in turn eliminate the smallest (unreadable value) and provide a larger max size.

Comment 1

[Blake Winton (:bwinton) (:☕️)]

Before:
  Smallest - https://www.dropbox.com/s/e40fh39md161ugi/Screenshot%202015-04-24%2014.31.42.png?dl=0
  Largest - https://www.dropbox.com/s/w979iakj3aq5g46/Screenshot%202015-04-24%2014.32.12.png?dl=0

After:
  Smallest - https://www.dropbox.com/s/lrib8cfo3rro7no/Screenshot%202015-04-24%2014.30.51.png?dl=0
  Largest - https://www.dropbox.com/s/k7e5f8emuo3kdz6/Screenshot%202015-04-24%2014.31.06.png?dl=0

Comment 2

[Blake Winton (:bwinton) (:☕️)]

Attachment: The first cut at the patch.

Comment 3

[Marco Mucci [:MarcoM]]

Hi Blake, can you provide a point value.

Comment 4

[:Margaret Leibovic]

Comment on attachment 8597529 [details] [diff] [review]
The first cut at the patch.

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

::: toolkit/themes/shared/aboutReader.css
@@ +53,5 @@
>  body.serif .remove-button  {
>    font-family: Georgia, "Times New Roman", serif;
>  }
>  
> +#container.font-size1 {

Why did you need to change this from body to #container?

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Because _setFontSize sets the font-sizeN on the container, not the body, at https://dxr.mozilla.org/mozilla-central/source/toolkit/components/reader/AboutReader.jsm#362  (And that was changed so that we didn't repaint the toolbar when we changed the font-size as part of bug 1149520.)  I'm not entirely sure how the code was working before…

Comment 6

[:Margaret Leibovic]

(In reply to Blake Winton (:bwinton) from comment #5)
> Because _setFontSize sets the font-sizeN on the container, not the body, at
> https://dxr.mozilla.org/mozilla-central/source/toolkit/components/reader/
> AboutReader.jsm#362  (And that was changed so that we didn't repaint the
> toolbar when we changed the font-size as part of bug 1149520.)  I'm not
> entirely sure how the code was working before…

Doh, I just noticed this myself while working on the patch for bug 1158194. This must be a regression I introduced in bug 1154028.

Comment 7

[:Gijs (he/him)]

Oy. Sorry for missing that. :-\

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/2982f76606da

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2982f76606da

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8597529 [details] [diff] [review]
The first cut at the patch.

Approval Request Comment
[Feature/regressing bug #]: reader mode
[User impact if declined]: Slightly harder to read text, and inability to resize text.
[Describe test coverage new/current, TreeHerder]: Manual
[Risks and why]: Low, because it's a CSS-only change.
[String/UUID change made/needed]: none

Comment 11

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8597529 [details] [diff] [review]
The first cut at the patch.

Approving for uplift to aurora and to beta (from discussion with Sylvestre) because we want this to be in the  38.0.5 but not in 38.0 since Reader Mode is disabled for 38.0 on Desktop.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d1ec9f37d8f5

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/8cba8416a229

Comment 14

[:Gijs (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #13)
> https://hg.mozilla.org/releases/mozilla-release/rev/8cba8416a229

Isn't this 38, and beta 38.0.5 ? Did I miss another memo? :-\

(neither were marked 'fixed' with this comment, which is presumably also an issue)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

beta merged to release yesterday

Comment 16

[Ashickur Rahman]

Towkir please mention which build did you check the fix.

Comment 17

[[:Towkir] Ahmed]

build of the fixed version is  :   20150521175336

more details at  http://prntscr.com/7qld12

Thanks :)