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)
Tracking
(firefox18 unaffected, firefox19+ fixed, firefox20+ fixed, firefox21 fixed)
RESOLVED
FIXED
Firefox 21
People
(Reporter: kbrosnan, Assigned: kats)
References
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
blassey
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
Comment 1•12 years ago
|
||
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
Comment 2•12 years ago
|
||
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.
Comment 3•12 years ago
|
||
(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.
Comment 4•12 years ago
|
||
(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.
Assignee | ||
Comment 5•12 years ago
|
||
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"
Comment 6•12 years ago
|
||
(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.
Updated•12 years ago
|
Summary: Reader mode not displayed for a simple document → Adjust low memory bar for Reader Mode
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #699831 -
Flags: review?(blassey.bugs)
Updated•12 years ago
|
Attachment #699831 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 9•12 years ago
|
||
Assignee: nobody → bugmail.mozilla
Comment 10•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Comment 11•12 years ago
|
||
Uplift potential?
Assignee | ||
Comment 12•12 years ago
|
||
Yeah, we can request uplift to aurora and beta.
Assignee | ||
Comment 13•12 years ago
|
||
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?
Comment 14•12 years ago
|
||
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.
Assignee | ||
Comment 15•12 years ago
|
||
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.
Comment 16•12 years ago
|
||
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).
Comment 17•12 years ago
|
||
(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`
Comment 18•12 years ago
|
||
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
Assignee | ||
Comment 19•12 years ago
|
||
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.
Comment 20•12 years ago
|
||
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
Comment 21•12 years ago
|
||
(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.
Reporter | ||
Comment 22•12 years ago
|
||
This does not need to be checked into beta. My regression range is valid and contains bug 792603.
Blocks: 792603
status-firefox18:
--- → unaffected
status-firefox19:
--- → unaffected
status-firefox20:
--- → affected
Assignee | ||
Comment 23•12 years ago
|
||
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.
Updated•12 years ago
|
Updated•12 years ago
|
Attachment #699831 -
Flags: approval-mozilla-beta?
Attachment #699831 -
Flags: approval-mozilla-beta+
Attachment #699831 -
Flags: approval-mozilla-aurora?
Attachment #699831 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 24•12 years ago
|
||
Updated•12 years ago
|
Comment 25•12 years ago
|
||
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
Assignee | ||
Comment 26•12 years ago
|
||
Good point, I will start a discussion thread on that article. Thanks for the tip!
Updated•11 years ago
|
tracking-fennec: ? → ---
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•