1349355 - Tapping "Aa" from the "Aa" context menu causes text to change size

Status: VERIFIED FIXED

(Firefox for iOS :: General, defect, P3)

Description

[Adrian Oniga]

Attachment: Aa.jpg

Steps to reproduce:

1.Open Firefox 
2.Go to bbc.com
3.Open an article in Reader Mode
4.Tap "Aa" and from the context menu tap "Aa" 



Actual results:

2.Page is displayed
3.Article is displayed in Reader Mode
4.Tapping "Aa" from the "Aa" context menu causes text to change size



Expected results:

4.Tapping "Aa" from the "Aa" context menu should not change the size of the text

Found on iPhone 6, iOS 10.2.1, Firefox version 8.0(1), buddybuild #2137

Comment 1

[Aaron Train [:aaronmt]]

Justin, since you're looking at a similar bug might as well look at this

Comment 2

[Justin D'Arcangelo [:justindarc]]

Taking this to investigate. Sounds like the Sans-serif/Serif bug.

Comment 3

[Justin D'Arcangelo [:justindarc]]

Attachment: GitHub Pull Request

This patch adds a slightly new behavior that I discussed with :tecgirl from UX where tapping the "Aa" will reset the font size in reader mode to the default size. This adds a new accessibility label to describe the behavior which I added to Strings.swift.

Comment 4

[Justin D'Arcangelo [:justindarc]]

I should also clarify, the problem in the original bug was caused by the -/+ button widths being 50% each and there was no "dead space" in between them. This is what caused the strange behavior where tapping "Aa" would seemingly randomly increase/decrease the font size.

Comment 5

[Stephan Leroux [:sleroux]]

Comment on attachment 8852189 [details] [review]
GitHub Pull Request

LGTM. We should file a follow up bug to add UI tests for this view. I don't think we have any for the reader view mode :/

Comment 6

[Isabel Rios[:isabel_rios]]

There is PR coming shortly to have ReaderViewUI tests enabled again (Bug 1348268). Although this particular case is not covered yet. Once the existing tests are enabled we will work on adding more and see if this check can be added too :)
And, there are also some UI Tests for the ReadingList view already running as UI.

Comment 7

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-mobile/firefox-ios/commit/f3513804a21da29cb18c3900855901bfcf1ee369

Comment 8

[Adrian Oniga]

Verified as fixed on iPhone 6, iOS 10.2.1, Firefox 8.0(1), buddybuild #2356