Open Bug 1417769 Opened 7 years ago Updated 2 years ago

collect onboarding-noshow-smallscreen and overlay-disappear-resize event for onboarding telemetry

Categories

(Firefox :: Tours, enhancement, P2)

Product:
Firefox
Component:
Tours
Type:
enhancement

Tracking

()

People

(Reporter: gasolin, Unassigned)

References

Details

(Whiteboard: [onboarding-telemetry] )

Attachments

(2 files)

Bug 1417769 - collect onboarding-noshow-smallscreen and overlay-disappear-resize event for onboarding telemetry;
59 bytes, text/x-review-board-request
francois
: review+
Details
data-review-request.md
2.40 KB, text/markdown
francois
: review+
Details 
From UX request, we want collect width to better evaluate for the responsive design.

We will collect current screen width when following events are triggered
* onboarding-logo-click
* notification-cta-click
* onboarding-noshow events.

And we want to add 2 new events
* onboarding-noshow
  event is sent when user is in a smaller screen that we won't show the onboarding
* overlay-disapear
  event is sent when user open the overlay and resized to a smaller window
Whiteboard: [onboarding-telemetry]
Summary: add width column and collect related events for on boarding telemetry → collect onboarding-noshow and overlay-disapear event for onboarding telemetry
We need a separate data review request here for the new `onboarding-noshow` and `overlay-disapear` events
renamed events to onboarding-noshow-smallscreen and overlay-disapear-resize
Summary: collect onboarding-noshow and overlay-disapear event for onboarding telemetry → collect onboarding-noshow-smallscreen and overlay-disapear-resize event for onboarding telemetry
Attached file data-review-request.md — 
ask data-review first, will add code review after data-review+


This bug will add 2 new events:

1. onboarding-noshow-smallscreen: if user open a new tab in a small screen (< 960px)
2. overlay-disapear-resize: if user open the overlay and resize it to small screen

All send column(data) are data-review+'d in Bug 1413830.

The send `width` is round to the nearest 50px (ex: 960px will be 950px when we get the width on database) to make tracking identity a little harder.
Attachment #8934385 - Flags: review?(francois)
Comment on attachment 8934385 [details]
data-review-request.md

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

Yes, browser/extensions/onboarding/data_events.md

2) Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available.

Yes, disabling telemetry.

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

Fred Lin.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Category 1 or 2.

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

Default ON

6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No.

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

Yes.

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

No, permanent.
Attachment #8934385 - Flags: review?(francois) → review+
Comment on attachment 8932704 [details]
Bug 1417769 - collect onboarding-noshow-smallscreen and overlay-disappear-resize event for onboarding telemetry;

https://reviewboard.mozilla.org/r/203762/#review212070

datareview+
Attachment #8932704 - Flags: review?(francois) → review+
before bug 1413830 is landing, please apply bug 1413830 first to test on device.
Comment on attachment 8932704 [details]
Bug 1417769 - collect onboarding-noshow-smallscreen and overlay-disappear-resize event for onboarding telemetry;

https://reviewboard.mozilla.org/r/203762/#review217412

::: browser/extensions/onboarding/content/onboarding.js:433
(Diff revision 8)
> +        session_key: this._session_key,
> +        width: this._roundWindowWidth,
> +      });
> +      // Don't show the overlay UI before we get to a better, responsive design.
> +      this.destroy();
> +      return;

Assume the page loads --> onVisiable --> _startUI --> _isSmallScreen is true --> "onboarding-noshow-smallscreen" sent --> returned --> user resizes the window --> _isSmallScreen is false --> The onboarding  elements should appear but dosen't because didn't do `this._window.addEventListener("resize", this);`

::: browser/extensions/onboarding/content/onboarding.js:451
(Diff revision 8)
>      });
>    }
>  
>    _resizeUI() {
> -    this._windowWidth = this._window.document.body.getBoundingClientRect().width;
> -    if (this._windowWidth < ONBOARDING_MIN_WIDTH_PX) {
> +    if (this._isSmallScreen) {
> +      if (this._overlay && this._overlay.classList.contains("onboarding-opened")) {

This dosen't cover the case that the overlay is closed but the onboarding icon button exists.
Attachment #8932704 - Flags: review?(fliu)
Comment on attachment 8932704 [details]
Bug 1417769 - collect onboarding-noshow-smallscreen and overlay-disappear-resize event for onboarding telemetry;

https://reviewboard.mozilla.org/r/203762/#review217424

::: browser/extensions/onboarding/content/onboarding.js:433
(Diff revision 8)
> +        session_key: this._session_key,
> +        width: this._roundWindowWidth,
> +      });
> +      // Don't show the overlay UI before we get to a better, responsive design.
> +      this.destroy();
> +      return;

thanks for catching this. Changed the order so we register unload/resize event before checking the small window size

::: browser/extensions/onboarding/content/onboarding.js:451
(Diff revision 8)
>      });
>    }
>  
>    _resizeUI() {
> -    this._windowWidth = this._window.document.body.getBoundingClientRect().width;
> -    if (this._windowWidth < ONBOARDING_MIN_WIDTH_PX) {
> +    if (this._isSmallScreen) {
> +      if (this._overlay && this._overlay.classList.contains("onboarding-opened")) {

As bryant's request, UX only interest in the case when overlay is opened and disappeared when user resize the window to a too small size. See reference in data_events.md
> As bryant's request, UX only interest in the case when overlay is opened and
> disappeared when user resize the window to a too small size. See reference
> in data_events.md

Bryant, could you help double confirm if this behavior (only catch the event when the overlay is shown and user resize to a too small window and let the overlay disappear) is what you expect?
Flags: needinfo?(bmao)
Summary: collect onboarding-noshow-smallscreen and overlay-disapear-resize event for onboarding telemetry → collect onboarding-noshow-smallscreen and overlay-disappear-resize event for onboarding telemetry
Comment on attachment 8932704 [details]
Bug 1417769 - collect onboarding-noshow-smallscreen and overlay-disappear-resize event for onboarding telemetry;

https://reviewboard.mozilla.org/r/203762/#review217716

::: browser/extensions/onboarding/content/onboarding.js:438
(Diff revision 10)
> +        session_key: this._session_key,
> +        width: this._roundWindowWidth,
> +      });
> +      // Don't show the overlay UI before we get to a better, responsive design.
> +      this.destroy();
> +      return;

Have to take care of "onboarding-session-begin" ping too, right now, the ping will not be sent if return here
Attachment #8932704 - Flags: review?(fliu)
Assignee: gasolin → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(bmao)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: