Closed Bug 767599 Opened 9 years ago Closed 9 years ago

Disable bookmarking while in reader mode

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 16

People

(Reporter: aaronmt, Unassigned)

References

Details

Attachments

(1 file)

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.
Summary: Toss out syncing of about:reader?url=n URL's? → Disable bookmarking while in reader mode
Blocks: reader
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
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
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
(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?
tracking-fennec: ? → ---
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.