Closed Bug 1134361 Opened 5 years ago Closed 5 years ago

Tip for adding articles to reading list is wrong for low-memory devices

Categories

(Firefox for Android Graveyard :: Reading List, defect)

35 Branch
All
Android
defect
Not set

Tracking

(firefox38 fixed, firefox39 fixed, fennec38+)

RESOLVED FIXED
Firefox 39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed
fennec 38+ ---

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(1 file, 1 obsolete file)

With bug 1123102, we now always show the reading list on all devices, but we still don't automatically parse pages to show the reader mode button on low memory devices.

While not incorrect, the tip in the empty view isn't really useful for low-memory device users, since they'll never see that reader mode button.

This goes hand-in-hand with bug 1127445. When we land something there, we should update the tip. Alternately, we could always hide this tip on low-memory devices, or just get rid of it.
tracking-fennec: --- → ?
Let's just hide the Tip for low memory devices.
Assignee: nobody → margaret.leibovic
tracking-fennec: ? → 38+
/r/5057 - Bug 1134361 - Hide "tip" to add to reading list in reading list panel on low memory devices. r=nalexander

Pull down this commit:

hg pull review -r 8b1050a149548a09ced278be2b416fb4c1ba7efe
Attachment #8574991 - Flags: review?(nalexander)
Sadly, I had to add back the Java-side low memory logic that I removed in bug 1123102.
Comment on attachment 8574991 [details]
MozReview Request: bz://1134361/margaret

https://reviewboard.mozilla.org/r/5055/#review4081

::: mobile/android/base/home/ReadingListPanel.java
(Diff revision 1)
> +            if (HardwareUtils.isLowMemoryPlatform()) {

To be clear: we're deciding whether to offer "Reader mode/add to RL" in JS, which depends on the low mem option, so we need to reflect this in the Java side?
Attachment #8574991 - Flags: review?(nalexander)
(In reply to Nick Alexander :nalexander from comment #4)
> Comment on attachment 8574991 [details]
> MozReview Request: bz://1134361/margaret
> 
> https://reviewboard.mozilla.org/r/5055/#review4081
> 
> ::: mobile/android/base/home/ReadingListPanel.java
> (Diff revision 1)
> > +            if (HardwareUtils.isLowMemoryPlatform()) {
> 
> To be clear: we're deciding whether to offer "Reader mode/add to RL" in JS,
> which depends on the low mem option, so we need to reflect this in the Java
> side?

Correct. We only show the reader view book icon in the toolbar if the device has enough memory for us to do the background parsing (determined in JS). So low-memory devices will never see this icon, so we should hide this tip.

With bug 1127445, those users will be able to add things to their reading list from the menu.
https://hg.mozilla.org/mozilla-central/rev/86b6e026fbc4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Attachment #8574991 - Attachment is obsolete: true
Attachment #8619515 - Flags: review+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.