Disable bookmarking while in reader mode

RESOLVED FIXED in Firefox 16

Status

()

Firefox for Android
General
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: aaronmt, Unassigned)

Tracking

unspecified
Firefox 16
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Currently under Nightly (06.22) with reader mode one may add a bookmark with the URI scheme: about:reader?url=<site> e.g, url=http://www.mozilla.com

Should these be tossed out of Syncing until desktop Firefox gets about:reader? Otherwise these are kind of duds on desktop.
If these are parented directly under "readinglist", then we already ignore them.
(In reply to Aaron Train [:aaronmt] from comment #0)
> Currently under Nightly (06.22) with reader mode one may add a bookmark with
> the URI scheme: about:reader?url=<site> e.g, url=http://www.mozilla.com
> 
> Should these be tossed out of Syncing until desktop Firefox gets
> about:reader? Otherwise these are kind of duds on desktop.

Yeah, we should probably disable bookmarking while in reader mode.

Updated

6 years ago
Summary: Toss out syncing of about:reader?url=n URL's? → Disable bookmarking while in reader mode

Updated

6 years ago
Blocks: 696921
sync triage: disabling bookmarking is a fennec proper thing, moving over to fennec
Component: Android Sync → General
Product: Mozilla Services → Fennec Native
QA Contact: android-sync → general
Created attachment 637420 [details] [diff] [review]
Disable bookmarking while in reader mode
Attachment #637420 - Flags: review?(mark.finkle)
Comment on attachment 637420 [details] [diff] [review]
Disable bookmarking while in reader mode

Is this enough? Do we want to show the state as "checked" or "not checked"?
Maybe a disabled menuitem does not show the checked state
(In reply to Mark Finkle (:mfinkle) from comment #6)
> Maybe a disabled menuitem does not show the checked state

Well, the bookmark menu item will never be in checked state and disabled because reader can never be bookmarked in the first place.
Comment on attachment 637420 [details] [diff] [review]
Disable bookmarking while in reader mode


>-        bookmark.setEnabled(true);
>+        bookmark.setEnabled(!tab.getURL().startsWith("about:reader"));

OK, this appears safe because we null-check tab.getURL above:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#579
Attachment #637420 - Flags: review?(mark.finkle) → review+
Lucas, what if we bookmark the original URL when in reader mode? So bookmarking "about:reader?url=http://www.example.com" would create a new bookmark for "http://www.example.com".
https://hg.mozilla.org/mozilla-central/rev/3fadf91276fc
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16

Updated

6 years ago
status-firefox16: affected → ---
(In reply to Chris Peterson (:cpeterson) from comment #10)
> Lucas, what if we bookmark the original URL when in reader mode? So
> bookmarking "about:reader?url=http://www.example.com" would create a new
> bookmark for "http://www.example.com".

Good point. Just wonder if that's what users would expect when bookmarking while on reader though. Ian, Madhava, thoughts?
(Assignee)

Updated

5 years ago
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.