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

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Metrics, defect)

Description

[Barbara Bermes [:barbara] - NI please!]

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.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

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?

Comment 2

[:Margaret Leibovic]

(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.

Comment 3

[Andrzej Hunt :ahunt]

Attachment: MozReview Request: Bug 1246712 - distinguish reader view pages when bookmarking r?margaret

Review commit: https://reviewboard.mozilla.org/r/45609/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45609/

Comment 4

[:Margaret Leibovic]

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.

Comment 5

[Andrzej Hunt :ahunt]

https://hg.mozilla.org/integration/fx-team/rev/88a642111cc13023c64ecca920312e1b3e8b372c
Bug 1246712 - distinguish reader view pages when bookmarking r=margaret

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/88a642111cc1