1155177 - Reader View controls bar can be zoomed in if 'Request Desktop site' option is enabled

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Reader View, defect)

Description

[Flaviu Cos]

Attachment: zoomed controls bar in reader view.png

Environment: 
Device: Asus Transformer Tab (Android 4.2.1);
Build: Nightly 40.0a1 (2015-04-16);

Steps to reproduce:
1. Launch Fennec;
2. Go to a website (e.g.: http://en.wikipedia.org/wiki/Mozilla);
3. Enable 'Request Desktop Site from the options menu;
4. Enable Reader View;
5. Zoom in the reading content.

Expected result:
Reader View can not be zoomed in or out.

Actual result:
Zooming in the page in Reader View will also zoom in the Reader View controls bar.

Notes:
Please check the attached screenshot.

Comment 1

[Flaviu Cos]

Inbound regression window:

Last good revision: 2d5eaa85e9da
First bad revision: 730f2da2e733
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2d5eaa85e9da&tochange=730f2da2e733

Comment 2

[:Margaret Leibovic]

kats, is there any way we can special-case reader view's viewport?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Probably. What would you like the behaviour here to be - do you always want to render reader mode with a mobile viewport, even if the page was being viewed in desktop mode? Another alternative would be to render the reader view in desktop mode but just disable zooming.

Comment 4

[:Margaret Leibovic]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Probably. What would you like the behaviour here to be - do you always want
> to render reader mode with a mobile viewport, even if the page was being
> viewed in desktop mode? Another alternative would be to render the reader
> view in desktop mode but just disable zooming.

I think we should just always render reader mode with a mobile viewport, since I don't see any benefit to "desktop mode" for reader view. The fact that reader view is an HTML page should just be an implementation detail - the user shouldn't think of it as part of the web.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Sounds good.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Comment 7

[:Margaret Leibovic]

Comment on attachment 8593612 [details] [diff] [review]
Patch

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

Looks fine to me. Thanks for the quick fix!

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6726887ce0d2

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6726887ce0d2

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8593612 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: bug 1106905
[User impact if declined]: when going into reader view while desktop mode is on, makes reader view render in desktop mode and be zoomable which is not nice
[Describe test coverage new/current, TreeHerder]: locally; no automated tests
[Risks and why]: fairly low risk, simple change
[String/UUID change made/needed]: none

Comment 11

[Flaviu Cos]

Verified as fixed in build 40.0a1 2015-04-20;
Device: Asus Transformer Tab (Android 4.2.1);

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8593612 [details] [diff] [review]
Patch

Verified on Nightly. Let's get the fix onto Aurora. Aurora+

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[Flaviu Cos]

Verified as fixed in build 39.0a2 (2015-04-23);
Asus Transformer Tab (Android 4.2.1)