Closed Bug 1158294 Opened 5 years ago Closed 4 years ago

Increase Reader Views Default Type Size

Categories

(Firefox :: General, defect)

38 Branch
defect
Not set
Points:
1

Tracking

()

RESOLVED FIXED
Firefox 40
Iteration:
40.2 - 27 Apr
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: mmaslaney, Assigned: bwinton)

References

Details

Attachments

(1 file)

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.
Attachment #8597529 - Flags: ui-review?(mmaslaney)
Attachment #8597529 - Flags: review?(margaret.leibovic)
Hi Blake, can you provide a point value.
Status: NEW → ASSIGNED
Iteration: --- → 40.2 - 27 Apr
Flags: qe-verify-
Flags: needinfo?(bwinton)
Flags: firefox-backlog+
Points: --- → 1
Flags: needinfo?(bwinton)
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?
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…
Attachment #8597529 - Flags: ui-review?(mmaslaney) → ui-review+
(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.
Attachment #8597529 - Flags: review?(margaret.leibovic) → review+
Blocks: 1154028
Keywords: checkin-needed
Oy. Sorry for missing that. :-\
https://hg.mozilla.org/mozilla-central/rev/2982f76606da
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
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
Attachment #8597529 - Flags: approval-mozilla-beta?
Attachment #8597529 - Flags: approval-mozilla-aurora?
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.
Attachment #8597529 - Flags: approval-mozilla-beta?
Attachment #8597529 - Flags: approval-mozilla-beta+
Attachment #8597529 - Flags: approval-mozilla-aurora?
Attachment #8597529 - Flags: approval-mozilla-aurora+
Target Milestone: Firefox 38 → Firefox 40
(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)
Flags: needinfo?(ryanvm)
beta merged to release yesterday
Flags: needinfo?(ryanvm)
Status: RESOLVED → VERIFIED
Towkir please mention which build did you check the fix.
Status: VERIFIED → RESOLVED
Closed: 4 years ago4 years ago
Flags: needinfo?(towkir17)
build of the fixed version is  :   20150521175336

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

Thanks :)
Flags: needinfo?(towkir17)
Duplicate of this bug: 1159684
You need to log in before you can comment on or make changes to this bug.