Closed Bug 828124 Opened 12 years ago Closed 12 years ago

Adjust low memory bar for Reader Mode

Categories

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

ARM
Android
defect
Not set
normal

Tracking

(firefox18 unaffected, firefox19+ fixed, firefox20+ fixed, firefox21 fixed)

RESOLVED FIXED
Firefox 21
Tracking Status
firefox18 --- unaffected
firefox19 + fixed
firefox20 + fixed
firefox21 --- fixed

People

(Reporter: kbrosnan, Assigned: kats)

References

Details

(Keywords: regression)

Attachments

(2 files)

Attached file test case (deleted) —
Reader mode is not displayed on my G2 running Android 2.3.7. Document consists of html header info then <p><p>Some text</p> <p>second paragraph</p> The regression range for this is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=20ec9014f220&tochange=d8e4f06198dc
Is this really a regression? Also, we need to check to see if the document is a "valid" read-able document. I don't think it is by the look of it.
Component: General → Reader Mode
I have not changed anything in reader mode for some time but I've heard reports from some users that reader mode is being offered at all in Nightly but works fine in, say, Beta.
(In reply to Lucas Rocha (:lucasr) from comment #2) > I have not changed anything in reader mode for some time but I've heard > reports from some users that reader mode is being offered at all in Nightly > but works fine in, say, Beta. This might be happening due to the "isLowMemoryPlatform" check we do here: http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3449 For example, this check stops Reader Mode, for all situations, on my Nexus S device.
(In reply to Mark Finkle (:mfinkle) from comment #3) > (In reply to Lucas Rocha (:lucasr) from comment #2) > > I have not changed anything in reader mode for some time but I've heard > > reports from some users that reader mode is being offered at all in Nightly > > but works fine in, say, Beta. > > This might be happening due to the "isLowMemoryPlatform" check we do here: > http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/ > browser.js#3449 > > For example, this check stops Reader Mode, for all situations, on my Nexus S > device. Is the Nexus S supposed to be considered a "low memory" device? I'd expect it to run fine with reader mode.
The "low memory" bar is set to 512 MiB as reported by the MemTotal value in /proc/meminfo. Most devices that are supposedly 512-meg devices report a value that is smaller than 512 in their MemTotal (presumably because some is taken up by radio firmware and/or graphics) and so they are considered "low memory". For example mfinkle's Nexus S reports 406052 kB even though it's a 512-meg device. I can lower the "low mem" threshold to 384 or so to account for this. Actual 384-meg devices tend to report just under 300 as their MemTotal so they would still be "low memory"
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5) > The "low memory" bar is set to 512 MiB as reported by the MemTotal value in > /proc/meminfo. Most devices that are supposedly 512-meg devices report a > value that is smaller than 512 in their MemTotal (presumably because some is > taken up by radio firmware and/or graphics) and so they are considered "low > memory". For example mfinkle's Nexus S reports 406052 kB even though it's a > 512-meg device. I can lower the "low mem" threshold to 384 or so to account > for this. Actual 384-meg devices tend to report just under 300 as their > MemTotal so they would still be "low memory" Makes sense to lower the threshold then.
Summary: Reader mode not displayed for a simple document → Adjust low memory bar for Reader Mode
Attached patch Adjust low-memory threshold (deleted) — Splinter Review
Attachment #699831 - Flags: review?(blassey.bugs)
Attachment #699831 - Flags: review?(blassey.bugs) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Uplift potential?
Yeah, we can request uplift to aurora and beta.
Comment on attachment 699831 [details] [diff] [review] Adjust low-memory threshold [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 801818, bug 808772 User impact if declined: Some "512 meg" devices get treated as low-memory devices, and don't get offered reader mode. They will also be subject to more aggressive tab expiration. Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): low risk, android only String or UUID changes made by this patch: none
Attachment #699831 - Flags: approval-mozilla-beta?
Attachment #699831 - Flags: approval-mozilla-aurora?
Be aware that the reported memory is no constant: On a Nexus S with Android 4.1.2 stock, I can see no Reader icon or menu items with Nightly 20130110 and 'Manage apps' shows 343 MB RAM.
The 'Manage apps' and any other view of memory in the Android UI is highly inconsistent. The memory reading we take is what the kernel provides as being visible, which should be the same across all instances of a given hardware model.
Unfortunately, running |adb shell| and calling /proc/meminfo throws a 'Permission denied.' on a stock rooted Nexus S and if you run |adb root|, it will tell you that it can't do that on production builds. (The pie memory chart in ddms seems to have even lower memory reports).
(In reply to Archaeopteryx [:aryx] from comment #16) > Unfortunately, running |adb shell| and calling /proc/meminfo throws a > 'Permission denied.' on a stock rooted Nexus S and if you run |adb root|, it > will tell you that it can't do that on production builds. (The pie memory > chart in ddms seems to have even lower memory reports). `cat /proc/meminfo`
Thank you. MemTotal: 351420 kB Everything: MemTotal: 351420 kB MemFree: 11868 kB Buffers: 2220 kB Cached: 70056 kB SwapCached: 0 kB Active: 202632 kB Inactive: 51500 kB Active(anon): 182272 kB Inactive(anon): 22696 kB Active(file): 20360 kB Inactive(file): 28804 kB Unevictable: 332 kB Mlocked: 0 kB SwapTotal: 0 kB SwapFree: 0 kB Dirty: 0 kB Writeback: 0 kB AnonPages: 182200 kB Mapped: 77488 kB Shmem: 22780 kB Slab: 19432 kB SReclaimable: 3356 kB SUnreclaim: 16076 kB KernelStack: 5544 kB PageTables: 6880 kB NFS_Unstable: 0 kB Bounce: 0 kB WritebackTmp: 0 kB CommitLimit: 175708 kB Committed_AS: 3475088 kB VmallocTotal: 327680 kB VmallocUsed: 67704 kB VmallocChunk: 226308 kB
Huh, your MemTotal value is lower than what mfinkle's Nexus S reported. I wasn't expecting that. Do you know the model number of your Nexus S? Could be a slightly different hardware configuration.
Product name - herring HW version - rev 11 Bootloader version - i9020xxlc2 Baseband version - i9020xxki1 Carrier info - TMB Lock state - unlocked Model number - Nexus S Android version - 4.1.2 Kernel version - 3.0.31-g5894150 android-build@vpbs1 ) #1 PREEMPT Mon Sept 10 14:10:13 PDT 2012 Build number JZO54K
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #19) > Huh, your MemTotal value is lower than what mfinkle's Nexus S reported. I > wasn't expecting that. Do you know the model number of your Nexus S? Could > be a slightly different hardware configuration. We have to draw the line somewhere though. 384 seems like a good line.
This does not need to be checked into beta. My regression range is valid and contains bug 792603.
The reader mode issue isn't in beta, agreed, but I'd like the definition of a "low memory platform" to be consistent on all versions. Other features like the tab expiration also depend on this definition.
Attachment #699831 - Flags: approval-mozilla-beta?
Attachment #699831 - Flags: approval-mozilla-beta+
Attachment #699831 - Flags: approval-mozilla-aurora?
Attachment #699831 - Flags: approval-mozilla-aurora+
Preferrably this should be mentioned in SUMO? Especially this article: https://support.mozilla.org/en-US/kb/How%20to%20enable%20Reader%20Mode%20in%20Firefox%20for%20Android?esab=a&s=reader+mode&r=0&as=s It confused me because I used to see the reader mode button. Some other folks have seen this problem too: https://support.mozilla.org/en-US/questions/955736?esab=a&s=reader+mode&r=1&as=s
Good point, I will start a discussion thread on that article. Thanks for the tip!
tracking-fennec: ? → ---
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: