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

Status: RESOLVED FIXED

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

Description

[:Margaret Leibovic]

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.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's just hide the Tip for low memory devices.

Comment 2

[:Margaret Leibovic]

Attachment: MozReview Request: bz://1134361/margaret

/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

Comment 3

[:Margaret Leibovic]

Sadly, I had to add back the Java-side low memory logic that I removed in bug 1123102.

Comment 4

[Nick Alexander :nalexander [he/him]]

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?

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8574991 [details]
MozReview Request: bz://1134361/margaret

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

Ship It!

Comment 6

[:Margaret Leibovic]

(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.

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/86b6e026fbc4

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/86b6e026fbc4

Comment 9

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b8d06501b138

Comment 11

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1134361 - Hide "tip" to add to reading list in reading list panel on low memory devices. r=nalexander