Closed Bug 1550631 Opened 1 year ago Closed 4 months ago

Record telemetry for direct navigation to about:logins

Categories

(Firefox :: about:logins, enhancement, P2)

70 Branch
enhancement

Tracking

()

VERIFIED FIXED
Firefox 71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- disabled
firefox70 + verified
firefox71 --- verified

People

(Reporter: sfoster, Assigned: MattN)

References

(Depends on 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: [passwords:management] [skyline])

Attachments

(3 files, 1 obsolete file)

we want to record access to the password management UI via direct navigation (e.g. bookmark or entering about:logins into the address bar).

As of bug 1543499, each of the entry points from the browser chrome are instrumented. To include direct navigation we'll probably want to move the actual recordEvent call into the management UI itself, and pass the entry point in as arguments.

Jared, can you assign a priority and add this to your breakdown and bug tree?

Flags: needinfo?(jaws)
Depends on: 1549115
Flags: needinfo?(jaws)
Priority: -- → P2
Type: defect → enhancement
Flags: qe-verify+

(In reply to Sam Foster [:sfoster] (he/him) from comment #0)

we want to record access to the password management UI via direct navigation (e.g. bookmark or entering about:logins into the address bar).

As of bug 1543499, each of the entry points from the browser chrome are instrumented. To include direct navigation we'll probably want to move the actual recordEvent call into the management UI itself, and pass the entry point in as arguments.

It's unclear to me how we can do this. To differentiate bookmarks vs address bar, we would have to either rewrite stored bookmarks to include an origin argument, or add special code to various places to inject the argument dynamically[1][2].

Did you have any other ideas of how this could be implemented?

[1] https://searchfox.org/mozilla-central/rev/0da35261b6789eec65476dbdd4913df6e235af6d/browser/base/content/browser-places.js#713-717
[2] https://searchfox.org/mozilla-central/rev/0da35261b6789eec65476dbdd4913df6e235af6d/browser/base/content/browser-places.js#802-805

Flags: needinfo?(sfoster)

I think the idea was to have the browser-chrome entry points load something like about:logins?entryPoint=foo, and have about:logins do the recordEvent. If there's no entryPoint arg, we'll just put the recorded event in a direct access / entryPoint undefined bucket. I don't think we need to distinguish access via boomarks from entering directly into the address bar - sorry if that was implied in my wording of comment #0

Flags: needinfo?(sfoster)

I see, that makes more sense and is something we would be able to implement within our schedule.

Whiteboard: [passwords:management] [skyline]

cosmin, can we make sure this gets tested? Thanks!

Noting that from conversation with ddurst, this will likely land in beta 70.

Matt, just a note we are heading into beta 8 (of 14) now. Is this still planned for 70 release?
If it is aimed at 71 I can go ahead and move the tracking flag to 71.

Flags: needinfo?(MattN+bmo)

It is still aimed at 70. I'm looking into it.

Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(MattN+bmo)

Data review was already completed in attachment 9064121 [details]. Nothing has changed about those answers, this just adds one more "object" and moves recording to the content process when about:logins is enabled.

Edit: deleted duplicate post caused by a BMO bug.

Attached file data collection review (obsolete) —

We'd like to land this ASAP for uplift into 70.

Attachment #9094188 - Flags: data-review?(chutten)
Attachment #9094188 - Attachment is obsolete: true
Attachment #9094188 - Flags: data-review?(chutten)
Attachment #9094271 - Flags: data-review?(chutten)
Comment on attachment 9094271 [details]
data collection review v2

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry so is documented in its definitions file [Events.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Events.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, sfoster et al are responsible.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, Interaction.

    Is the data collection request for default-on or default-off?

Default on for all channels.

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

No. This collection is permanent.

---
Result: datareview+
Attachment #9094271 - Flags: data-review?(chutten) → data-review+

Matt, we could try to hustle to get this into the beta 9 build on Monday but it may be wiser to aim it at beta 10. If this lands today or over the weekend, and you feel confident it is working (or can let me know how we test and we don't see any obvious regressions) then we could try to uplift on Sunday evening / morning in the EU. Let me know as I'll be checking in periodically over the weekend. If that is too much of a scramble or doesn't work out then let's assume beta 10.

Flags: needinfo?(MattN+bmo)
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/889be6c334ae
Record telemetry for direct navigation to about:logins. r=jaws
https://hg.mozilla.org/integration/autoland/rev/87ed1b3f9b4f
Include the entryPoint for navigations from about:protections to about:logins. r=jaws
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/ec63d599b81e
Record telemetry for direct navigation to about:logins. r=jaws
https://hg.mozilla.org/integration/autoland/rev/571a8776eb3f
Include the entryPoint for navigations from about:protections to about:logins. r=jaws
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Comment on attachment 9094081 [details]
Bug 1550631 - Record telemetry for direct navigation to about:logins. r=jaws

Beta/Release Uplift Approval Request

  • User impact if declined: Won't be counting the number of times about:logins is opened from the protection report or the URL (via the URL bar, bookmarks, etc.) so we won't know which entry points are valuable/discoverable.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Try open the login management UI from all entry points ("aboutprotections", "autocomplete", "contextmenu", "fxamenu", "mainmenu", "pageinfo", "preferences") and loading the about:logins from the address bar or bookmark (aka. "direct"ly) and ensure that the page loads fine. Optionally confirm that the event telemetry is recorded appropriately (once per open) for each method of opening.
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): To avoid double-counting of a load as both a UI entry point and a direct URL load, we need to do a redirect to remove any entryPoint query params and that change was a bit risky as it happens before the page is rendered so if it causes an exception the page may not load properly. There aren't too many possible exceptions so this should become obvious within a few days on Nightly or from QA testing.
  • String changes made/needed: None
Flags: needinfo?(MattN+bmo)
Attachment #9094081 - Flags: approval-mozilla-beta?
Attachment #9094255 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]

I have verified this issue on the latest Nightly 71.0a1 (Build ID 20190924094348) build on Windows 7, MacOS 10.14 and Arch 4.14.

  • The telemetry is correctly recorded when navigation directly to about:logins using the following methods:
    • When navigating directly to about:logins or from the bookmark.
    • When navigating from "about:protections" page.
    • When navigating from autofill.
    • When navigating from the context menu of login fields.
    • When navigating from Firefox Account menu.
    • When navigating from Hamburger main menu.
    • When navigating from Page Info window.
    • When navigating from "about:preferences".
Status: RESOLVED → VERIFIED
Regressions: 1583625

Comment on attachment 9094081 [details]
Bug 1550631 - Record telemetry for direct navigation to about:logins. r=jaws

Adds telemetry for skyline, verified in nightly.
OK for uplift for beta 10.

Attachment #9094081 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9094255 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I have verified this issue on the Firefox Beta 70.0b10 (Build ID: 20190926005616) build on Windows 10, MacOS 10.14 and Ubuntu 16.04 x64.

  • The telemetry is correctly recorded when navigation directly to about:logins using the following methods:
    • When navigating directly to about:logins or from the bookmark.
    • When navigating from "about:protections" page.
    • When navigating from autofill.
    • When navigating from the context menu of login fields.
    • When navigating from Firefox Account menu.
    • When navigating from Hamburger main menu.
    • When navigating from Page Info window.
    • When navigating from "about:preferences".
Component: Password Manager → about:logins
Product: Toolkit → Firefox
Target Milestone: mozilla71 → Firefox 71
Version: unspecified → 70 Branch
You need to log in before you can comment on or make changes to this bug.