1200402 - Reader mode pages can get load with a desktop mode viewport

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

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

STR:
1. Long-press on a link and add it to your reading list (most things should do as long as they have wide content)
2. Open the thing you added to your reading list, it will get opened in reader mode
3. From the menu toggle "Request desktop site" so that it is enabled
4. Reload the page

Expected:
In steps 2 and 4 the page should get loaded with a mobile viewport (i.e. not horizontally scrollable)

Actual:
In step 4 the page gets loaded with a desktop viewport and is horizontally scrollable.

This is a regression from switching Fennec over to using MobileViewportManager. In Fennec about:reader is special-cased at [1] so that the desktop-mode viewport doesn't apply to it. But after the switch to MobileViewportManager this code path doesn't get used and the equivalent path at [2] doesn't have this special check.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=2d3464d58ff4#6384
[2] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp?rev=975b92a32949#7905

Comment 1

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

Attachment: Patch

Feel free to throw the review to somebody else. I decided to ignore "desktop mode" for all about: pages because then I don't have to hard-code about:reader, and also because I think it makes more sense in general.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

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

I think you need another review, but it seems fine to me. I don't know who the least-overloaded DOM peer is...but let's try Kyle?

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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

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

lol, you think I'm the least overloaded peer?

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40194ca33785

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/40194ca33785

Comment 6

[u549602]

Verified as fixed on latest Nightly