Closed Bug 872005 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox21 unaffected, firefox22 unaffected, firefox23 verified, firefox24 verified, relnote-firefox 23+)

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

People

(Reporter: aaronmt, Assigned: Margaret)

References

Details

Attachments

(1 file)

patch to remove context menu item
(deleted), patch
mfinkle
: review+
lsblakk
: approval-mozilla-aurora+
Details | Diff | Splinter Review
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+ → ---
status-firefox21: affected → unaffected
status-firefox22: affected → unaffected
Flags: needinfo?(aaron.train)
https://hg.mozilla.org/mozilla-central/rev/ee75d69fe879
Status: NEW → RESOLVED
Closed: 12 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
status-firefox23: fixed → verified
status-firefox24: fixed → verified
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

Creator:
Created:
Updated:
Size: