Closed
Bug 1231792
Opened 8 years ago
Closed 8 years ago
Experiment: Add bookmarks/history to 3-dot menu
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox45+ fixed, firefox46+ verified, firefox47+ verified, firefox48 verified, relnote-firefox 46+, fennec46+)
People
(Reporter: mcomella, Assigned: Margaret)
References
Details
Attachments
(2 files, 4 obsolete files)
140.00 KB,
image/png
|
Details | |
12.39 KB,
patch
|
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
So they're easier to find.
Reporter | ||
Comment 1•8 years ago
|
||
Bug 1231792 - Add bookmarks & history list menu item strings. r=liuche
Reporter | ||
Comment 2•8 years ago
|
||
Bug 1231792 - Add Bookmarks and History 3-dot menu items. r=liuche
Reporter | ||
Comment 3•8 years ago
|
||
Anthony gave the okay irl and is okay to land it. Margaret, do you feel we should we A/B test it? Then again, what would our metrics be? Counts of uses of these panels? Still TODO: add telemetry.
Flags: needinfo?(margaret.leibovic)
Reporter | ||
Comment 4•8 years ago
|
||
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #3) > Anthony gave the okay irl and is okay to land it. > > Margaret, do you feel we should we A/B test it? Then again, what would our > metrics be? Counts of uses of these panels? A/B would be showing these items or not? That could be an interesting test. And yes, the metrics would be how often users are on these panels. We do have telemetry probes already in place to tell us how often people visit different panels. > Still TODO: add telemetry. Let's definitely land telemetry when we land this. Or will this automatically have telemetry from our current menu telemetry? I think we should hold off on landing this until after the merge, or land it behind a Nightly only flag.
Flags: needinfo?(margaret.leibovic)
Comment 6•8 years ago
|
||
https://reviewboard.mozilla.org/r/27579/#review24865 ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3311 (Diff revision 1) > + } Why do the other code blocks return true, but yours do not. Returning early seems good. ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3317 (Diff revision 1) > + This block should be wrapped in a Switchboard block to enable A/B testing and stagged rollout.
Comment 7•8 years ago
|
||
(In reply to :Margaret Leibovic from comment #5) > > Still TODO: add telemetry. > > Let's definitely land telemetry when we land this. Or will this > automatically have telemetry from our current menu telemetry? We will get telemetry from the menu code. > I think we should hold off on landing this until after the merge, or land it > behind a Nightly only flag. Using Switchboard means it will be disabled until we turn it on for Nightly.
Comment 10•8 years ago
|
||
Good stuff also in regards to A/B as this will be a different way of testing a feature and its success. I assume we should work with Alex to fill out one of his A/B testing templates. Alex, do you want to do that together?
Flags: needinfo?(bbermes) → needinfo?(adavis)
Comment 11•8 years ago
|
||
Hm... From a UX point of view it's an obvious quick win though. :) This is something I've seen an appetite for in the wild and something we've noticed from our competitive analysis. Do we need to A/B test this? Or, maybe we could land it behind a flag for now and get telemetry?
Comment 12•8 years ago
|
||
Getting a useful A/B test out of this requires thought. Note that using SwitchBoard for staged rollouts is still useful! Measuring is still useful! Telemetry is great! But it's not an A/B test, and we don't have to A/B test things just for fun. It has a cost. We can test absolute usage through normal UI telemetry. It's not all that useful measuring how much time users spend on the bookmarks panel; time on the bookmarks panel isn't valuable. We can assume that users will spend _some_ amount of additional time if we add a menu item, precisely because we can only get a positive number of clicks. If we're A/B testing bookmark discovery we need: * A useful definition of success. I'm not convinced that "visits to bookmarks panel" is that. Perhaps zero-to-tap-bookmark for affected subsets of sessions is? Is there a way we can measure retention for this population? Do users with the menu create or visit more bookmarks? Do they do so more than people who *do* use bookmarks already via swiping to the panel? (We think that bookmarks are hard to find, not just high-friction.) * Useful alternatives to compare so we can understand a baseline. "No menu item" isn't really a useful alternative -- we're confident that the menu item will be a discoverability improvement, so we won't learn anything.
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #11) > Hm... From a UX point of view it's an obvious quick win though. :) This is > something I've seen an appetite for in the wild and something we've noticed > from our competitive analysis. > > Do we need to A/B test this? > > Or, maybe we could land it behind a flag for now and get telemetry? To be clear, I would want us to land this behind a switchboard "flag", not a Nightly flag. This will allow us to do a staged rollout and gather feedback. So far our main A/B test metric has been retention, which means we'd be asking "do these extra menu items lead to increased retention?" I'm not sure whether we need an A/B test for these items, but a staged rollout would be great. We have general telemetry for menu items, so I don't think we'll need any additional probes here. I think we should land this with switchboard controls in place, uplift it to beta, and see what a few beta users do when it's enabled.
Comment 14•8 years ago
|
||
https://reviewboard.mozilla.org/r/27579/#review25581 ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3192 (Diff revision 1) > return true; Probably add something like: if (SwitchBoard.isInExperiment(this, "panelmenuitems") == false) { bookmarksList.setVisible(false); historyList.setVisible(false); } Where "panelmenuitems" is the experiment name we need to add to mozilla-switchboard
Comment 15•8 years ago
|
||
(In reply to Barbara Bermes [:barbara] from comment #10) > Good stuff also in regards to A/B as this will be a different way of testing > a feature and its success. > > I assume we should work with Alex to fill out one of his A/B testing > templates. Alex, do you want to do that together? Yes Barbara, I think we should continue to work on these together.
Flags: needinfo?(adavis)
Comment 16•8 years ago
|
||
I'll setup a meeting in the new year for us to fill out one of your templates. Invite coming soon.
Comment 17•8 years ago
|
||
For measuring success, I would look at: -how many people see each variation (so we can establish rates and make sure both variations are being distributed properly) -how many times does history and bookmarks get used per variation (grand total). We will be able to see if users are using the features more. -for version B, how many times do people access bookmarks and history via menu vs panels when they have the choice. This will allow us to see if we are cannibalizing or reaching out to more users. -Number of times people bookmark. If accessing bookmarks is easier, perhaps people will bookmark more. If we can get these by unique users and absolute totals (count), it would give us the full portrait. But if we just have counts, we should still be able to determine success. We just won't know if more people are using, but rather that it is being used more. With this type of A/B test, we need to be sure that a user that is exposed to a specific variation continues to see only that variation. This is less important in firstrun since everyone just sees it once but for a feature, the UX needs to be consistent for the length of the test. Does Switchboard properly manage this or did make sure of this?
Flags: needinfo?(mark.finkle)
Comment 18•8 years ago
|
||
(In reply to Alex Davis [:adavis] from comment #17) > For measuring success, I would look at: > -how many people see each variation (so we can establish rates and make sure > both variations are being distributed properly) > -how many times does history and bookmarks get used per variation (grand > total). We will be able to see if users are using the features more. > -for version B, how many times do people access bookmarks and history via > menu vs panels when they have the choice. This will allow us to see if we > are cannibalizing or reaching out to more users. > -Number of times people bookmark. If accessing bookmarks is easier, perhaps > people will bookmark more. We have all of these metrics, via UI Telemetry > If we can get these by unique users and absolute totals (count), it would > give us the full portrait. But if we just have counts, we should still be > able to determine success. We just won't know if more people are using, but > rather that it is being used more. We only have absolute counts, via UI Telemetry. > With this type of A/B test, we need to be sure that a user that is exposed > to a specific variation continues to see only that variation. This is less > important in firstrun since everyone just sees it once but for a feature, > the UX needs to be consistent for the length of the test. Does Switchboard > properly manage this or did make sure of this? Yes. Switchboard manages this via it's segmentation method.
Flags: needinfo?(mark.finkle)
Comment 19•8 years ago
|
||
Sounds good. When it gets into nightly, let's make it a habit to just double check the tracking and make sure everything is there. We had a bit of trouble with the telemetry in our firstrun A/B test so we should approach with additional caution.
Assignee | ||
Comment 20•8 years ago
|
||
Let's see if I find time to update this patch before mcomella gets back to it.
Flags: needinfo?(margaret.leibovic)
Comment 21•8 years ago
|
||
(In reply to :Margaret Leibovic from comment #20) > Let's see if I find time to update this patch before mcomella gets back to > it. Something else to remember: start a UI Telemetry Session with the experiment name if a person uses either of the menus. We should end the session when they switch home panels [1] (or close the homepanels). You should be able to stop the session regardless if it's active or not. We ignore the call if the session is not active. Example of starting an experiment session [2] [1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/HomePager.java#525 [2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/firstrun/FirstrunPagerConfig.java#30
Assignee | ||
Comment 22•8 years ago
|
||
liuche said she would look at carrying this across the finish line. This has strings, so let's try to get it landed by the end of next week to avoid missing Fx46. Alternately, could we just use the same strings that we use for the home panel titles? Yes, they appear in different contexts, but they are very directly related. If we did this, then we could even uplift this change.
Assignee: michael.l.comella → liuche
Flags: needinfo?(margaret.leibovic)
Assignee | ||
Comment 25•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/31783/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/31783/
Attachment #8710470 -
Flags: review?(liuche)
Assignee | ||
Comment 26•8 years ago
|
||
This change also depends on changes to the switchboard server config. Review commit: https://reviewboard.mozilla.org/r/31785/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/31785/
Attachment #8710471 -
Flags: review?(liuche)
Assignee | ||
Comment 27•8 years ago
|
||
PR for the switchboard config changes: https://github.com/mozilla-services/switchboard-experiments/pull/1
Comment 28•8 years ago
|
||
https://reviewboard.mozilla.org/r/31785/#review28511 I only looked at some of the experiement aspects. ::: mobile/android/base/java/org/mozilla/gecko/home/HomePager.java:532 (Diff revision 1) > + Telemetry.startUISession(TelemetryContract.Session.EXPERIMENT, SwitchboardExperiments.BOOKMARKS_HISTORY_MENU); I think you meant Telemetry.stopUISession I am trying to think of how we want to analyze these types of experiment. Creating a session tags all events with that session name. What you have here should work OK. We'll be able to see any clicks or loadurls on the Bookmark or History panel that happened if the user displayed the panel via the menu. Makes sense. ::: mobile/android/base/java/org/mozilla/gecko/util/SwitchboardExperiments.java:9 (Diff revision 1) > + * https://github.com/leibovic/switchboard-experiments Use the real repo? ::: mobile/android/base/java/org/mozilla/gecko/util/SwitchboardExperiments.java:11 (Diff revision 1) > +public class SwitchboardExperiments { Could just be "Experiments" too
Assignee | ||
Updated•8 years ago
|
Attachment #8697373 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8697374 -
Attachment is obsolete: true
Assignee | ||
Comment 29•8 years ago
|
||
Comment on attachment 8710471 [details] MozReview Request: Bug 1231792 - Add Bookmarks and History 3-dot menu items. r=liuche Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31785/diff/1-2/
Assignee | ||
Comment 30•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe3d15411714
Comment 31•8 years ago
|
||
Comment on attachment 8710470 [details] MozReview Request: Bug 1231792 - Add bookmarks & history list menu item strings. r=liuche https://reviewboard.mozilla.org/r/31783/#review28727
Attachment #8710470 -
Flags: review?(liuche) → review+
Comment 32•8 years ago
|
||
Comment on attachment 8710471 [details] MozReview Request: Bug 1231792 - Add Bookmarks and History 3-dot menu items. r=liuche https://reviewboard.mozilla.org/r/31785/#review28739 This looks good to me - is there a follow-up to turn this on server-side? ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:65 (Diff revision 2) > -import org.mozilla.gecko.util.ActivityUtils; > +import org.mozilla.gecko.util.*; Our Java style doesn't support * imports - is this intentional?
Attachment #8710471 -
Flags: review?(liuche) → review+
Assignee | ||
Comment 33•8 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #32) > Comment on attachment 8710471 [details] > MozReview Request: Bug 1231792 - Add Bookmarks and History 3-dot menu items. > r=liuche > > https://reviewboard.mozilla.org/r/31785/#review28739 > > This looks good to me - is there a follow-up to turn this on server-side? That's the PR I linked above: https://github.com/mozilla-services/switchboard-experiments/pull/1 > ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:65 > (Diff revision 2) > > -import org.mozilla.gecko.util.ActivityUtils; > > +import org.mozilla.gecko.util.*; > > Our Java style doesn't support * imports - is this intentional? My IDE this this. Probably because we're actually using everything in the package. I can revert it manually, but this probably doesn't have any real performance impact (I believe that's the reason to avoid global imports).
Assignee | ||
Comment 34•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d7587396a9ad9d32ba2fd22bed4c479bda622608 Bug 1231792 - Add Bookmarks and History 3-dot menu items. r=liuche
Assignee | ||
Comment 35•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/2357fa4fe0a876b90704f690ca6d9e2dd4b8766d Bug 1231792 - Add Bookmarks and History 3-dot menu items. r=liuche
Assignee | ||
Comment 36•8 years ago
|
||
I landed a new version of the patch that doesn't add new strings, so that we can uplift this. The behavior is controlled by switchboard, so we could uplift this all the way to 45 and turn it on/off remotely.
Assignee | ||
Updated•8 years ago
|
Attachment #8710470 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8710471 -
Attachment is obsolete: true
Assignee | ||
Comment 37•8 years ago
|
||
Comment 38•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2357fa4fe0a8
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Assignee | ||
Comment 39•8 years ago
|
||
Comment on attachment 8711656 [details] [diff] [review] patch that landed This is breaking new ground for us - this feature is controlled by a switchboard flag, so by default, uplifting this patch won't have any affect on the UI. When we want to enable this on aurora/beta, we'll need to land a change to our switchboard experiments config: https://github.com/mozilla-services/switchboard-experiments I will work to create a process around this, but the policy should be that we won't make an experiment changes on aurora/beta/release without approval from relman and product. Approval Request Comment [Feature/regressing bug #]: None. [User impact if declined]: Hard to access history and bookmarks. [Describe test coverage new/current, TreeHerder]: No automated test coverage, tested locally. [Risks and why]: Low-risk, this adds two simple menu items, and it is disabled by default. There may be some risk around the fact that we're using Switchboard to control this, but I want to take this risk now, rather than when we have a more complex feature we absolutely must ship. [String/UUID change made/needed]: None.
Attachment #8711656 -
Flags: approval-mozilla-beta?
Attachment #8711656 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
status-firefox45:
--- → affected
status-firefox46:
--- → affected
Comment 40•8 years ago
|
||
Adding tracking flags (as I think we should track all experimental features)
Comment 41•8 years ago
|
||
Comment on attachment 8711656 [details] [diff] [review] patch that landed UI feature work, disabled by default, for switchboard experiment. Please uplift to aurora and beta. (Doing this while Sylvestre is on PTO, so it can make it to the Beta 2 build on Monday)
Attachment #8711656 -
Flags: approval-mozilla-beta?
Attachment #8711656 -
Flags: approval-mozilla-beta+
Attachment #8711656 -
Flags: approval-mozilla-aurora?
Attachment #8711656 -
Flags: approval-mozilla-aurora+
Comment 42•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/168efd14a44b
Comment 43•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/8d9e767ea418
Release Note Request (optional, but appreciated) [Why is this notable]: [Suggested wording]: Add “History” and “Bookmarks” to Menu [Links (documentation, blog post, etc)]:
relnote-firefox:
--- → ?
Comment 45•8 years ago
|
||
We have already this: 'Optimized & re-organized "Settings" under "Menu"' in the release notes
Assignee | ||
Comment 46•8 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #45) > We have already this: > 'Optimized & re-organized "Settings" under "Menu"' > in the release notes This is a different feature, and one that will be in Firefox 46, not 45. The settings reorg feature in 45 is about the settings, not the menu itself (maybe we should update the wording for that to be less confusing?).
Assignee | ||
Comment 49•8 years ago
|
||
We need to actually make a change to our switchboard config for this to appear in beta, I just filed bug 1253588 about that.
Comment 50•8 years ago
|
||
Verified as implemented on all versions : 48.0a1, 47.0a2, and 46.0b2
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•