Closed Bug 1246712 Opened 8 years ago Closed 8 years ago

Add telemetry probe to understand how often users save Reader Mode to Reading List (folder)

Categories

(Firefox for Android Graveyard :: Metrics, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: barbara, Assigned: ahunt)

References

Details

Attachments

(1 file)

For the upcoming migration work, we will need to make sure that this is included as well, and distinguishable, save as regular bookmark vs. save to reading list folder.
This will be a slippery slope, or at least "not obvious" from a probe perspective.

Pressing the "star" is adding a bookmark, so we should trigger a "save.1" event, but should we use "bookmark" or "reading_list" as the extra? The concept of a reading list will begin to get very blurry.

What is a readinglist or a bookmark folder?
(In reply to Mark Finkle (:mfinkle) from comment #1)
> This will be a slippery slope, or at least "not obvious" from a probe
> perspective.
> 
> Pressing the "star" is adding a bookmark, so we should trigger a "save.1"
> event, but should we use "bookmark" or "reading_list" as the extra? The
> concept of a reading list will begin to get very blurry.

I think this is a good idea. Fundamentally I think we're trying to understand how much people save reader view items. Let's put that directly in the UI action.
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Comment on attachment 8740172 [details]
MozReview Request: Bug 1246712 - distinguish reader view pages when bookmarking r?margaret

https://reviewboard.mozilla.org/r/45609/#review42415

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3410
(Diff revision 1)
>          if (itemId == R.id.bookmark) {
>              tab = Tabs.getInstance().getSelectedTab();
>              if (tab != null) {
> +                final String extra;
> +                if (AboutPages.isAboutReader(tab.getURL())) {
> +                    extra = "reading_list";

Maybe we should update this to be `"bookmark_reader"` instead, since this isn't actually adding anything to a reading list, but rather just bookmarking a reader view item (it's only later that we decide to show this in a "Reading List" folder).

::: mobile/android/base/java/org/mozilla/gecko/home/HomeFragment.java:389
(Diff revision 1)
> -                    Telemetry.sendUIEvent(TelemetryContract.Event.UNSAVE, TelemetryContract.Method.CONTEXT_MENU, "bookmark");
> +                    SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(mContext);
> +                    final boolean isReaderViewPage = rch.isURLCached(mUrl);
> +
> +                    final String extra;
> +                    if (isReaderViewPage) {
> +                        extra = "reading_list";

Same comment applies here.
Attachment #8740172 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/integration/fx-team/rev/88a642111cc13023c64ecca920312e1b3e8b372c
Bug 1246712 - distinguish reader view pages when bookmarking r=margaret
https://hg.mozilla.org/mozilla-central/rev/88a642111cc1
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.