Closed Bug 828124 Opened 11 years ago Closed 11 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
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
Attachment #699831 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/281657268a89
Status: NEW → RESOLVED
Closed: 11 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: