Closed Bug 872005 Opened 7 years ago Closed 7 years ago

Reader Mode: Able to add pages to reading list on devices incapable of accessing Reader Mode

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 24
Tracking Status
firefox21 --- unaffected
firefox22 --- unaffected
firefox23 --- verified
firefox24 --- verified
relnote-firefox --- 23+

People

(Reporter: aaronmt, Assigned: Margaret)

References

Details

Attachments

(1 file)

This kind of slipped by. One can long-tap and hold on a link and 'Add to Reading List' on devices that don't meet the minimum-bar requirement (see bug 792603 for details).

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#420
--
Samsung Nexus S (Android 4.1)
Nightly (05/14)
So the deal here is, a user can access the saved item in the reading list and is able to enter Reader Mode. There would be confusion because they're unable to access Reader Mode elsewhere via an article being parsed. Backdoor access to reader-mode. Should this be?
I'm seeing this on devices that support Reader Mode as well, and when you try to add something that can't be parsed by our Reader you get a fail message. It makes for pretty crummy UX.

We should pull it on lower end devices, and also on higher end devices until we have a better UX designed for adding things that Reader Mode can't parse.
Assignee: nobody → margaret.leibovic
tracking-fennec: ? → 22+
If I'm understanding Ian's last comment correctly, we want to get rid of this context menu item altogether until we can figure out a better UX for things we can't parse.

I didn't touch the string so that we can uplift this without string churn, but I can make another patch that gets rid of it for trunk.
Attachment #750767 - Flags: review?(mark.finkle)
Attachment #750767 - Flags: review?(mark.finkle) → review+
This context menu item was only added in 23... why did we mark this bug as status-firefox21/22:affected?
Blocks: 814587
Comment on attachment 750767 [details] [diff] [review]
patch to remove context menu item

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 814587
User impact if declined: users can add articles to their reading list on devices where we don't support reader mode
Testing completed (on m-c, etc.): tested locally
Risk to taking this patch (and alternatives if risky): low-risk, removes context menu item
String or IDL/UUID changes made by this patch: n/a
Attachment #750767 - Flags: approval-mozilla-aurora?
needinfo? to Aaron to ask about the status flags.
Flags: needinfo?(aaron.train)
(In reply to :Margaret Leibovic from comment #4)
> This context menu item was only added in 23... why did we mark this bug as
> status-firefox21/22:affected?

Confirmed. My mistake.
tracking-fennec: 22+ → ---
Flags: needinfo?(aaron.train)
https://hg.mozilla.org/mozilla-central/rev/ee75d69fe879
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Attachment #750767 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
flagging relnote to remind myself to put this in on or before we go to beta with 23.
relnote-firefox: --- → ?
Aurora 24.0a2 (2013-07-02)
Firefox for Android 23.0b2 (2013-07-02)
Device: LG Optimus 4x 
OS:Android 4.1.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.