Closed Bug 1756703 Opened 2 years ago Closed 2 years ago

PiP telemetry enhancements

Categories

(Toolkit :: Picture-in-Picture, enhancement, P2)

enhancement
Points:
5

Tracking

()

VERIFIED FIXED
100 Branch
Tracking Status
firefox100 --- verified

People

(Reporter: asafko, Assigned: niklas)

References

Details

(Whiteboard: [fidefe-MR1-2022])

Attachments

(5 files)

Probes that need to be revived or updated for Fx 100

pictureinpicture.closed
(event category: pictureinpicture, event method: closed_method) - let’s not revive the existing scalar, but add a new event with the following attributes instead:

closeButton
unpipButton
originTabClose
processShutdown

2. FX_PICTURE_IN_PICTURE_WINDOW_OPEN_DURATION

This is an existing expired histogram that needs to be revived.

3. pictureinpicture.create

(event category: pictureinpicture, event method: create) - this is an existing probe, but it needs two new attributes to help us track PiP windows created with subtitles on in the origin tab video and the “source” of subtitles (site-specific adapter or WebVTT):

subtitlesEnabled: bool
webVTTSubtitles: bool

4. pictureinpicture.settings.disable

(event category: pictureinpicture.settings, event method: disable)

This probe is currently only recorded when users uncheck Enable Picture-in-Picture video controls in Settings > Browse, but not when users right-click the PiP icon on the video and choose “Hide Picture-in-Picture toggle”.

We need to record the pictureinpicture.settings.disable event in the second scenario too.

Points: --- → 5
Attachment #9265602 - Flags: data-review?(chutten)

Comment on attachment 9265602 [details]
PiP Data Collection Request - Probes for PiP MR1.txt

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.

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, Ania Mininkova is 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 the data collection use a third-party collection tool?

No.


Result: datareview+

Attachment #9265602 - Flags: data-review?(chutten) → data-review+
Assignee: nobody → nbaumgardner
Status: NEW → ASSIGNED

Thank you very much for the review, Chris.

Pushed by nbaumgardner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ed10a16a22a
Create closed_method telemetry event for PiP. r=mconley
https://hg.mozilla.org/integration/autoland/rev/678fcec5884b
Revive FX_PICTURE_IN_PICTURE_WINDOW_OPEN_DURATION histogram. r=pip-reviewers,mconley
https://hg.mozilla.org/integration/autoland/rev/0fa2c4b2fc9a
Add extra keys to pictureinpicture.create telemetry event. r=pip-reviewers,mconley,kpatenio
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

4. pictureinpicture.settings.disable

(event category: pictureinpicture.settings, event method: disable)

This probe is currently only recorded when users uncheck Enable Picture-in-Picture video controls in Settings > Browse, but not when users right-click the PiP icon on the video and choose “Hide Picture-in-Picture toggle”.

We need to record the pictureinpicture.settings.disable event in the second scenario too.

I assume there was nothing to be fixed relating to this 4th part of the enhancement because it appears there was a misunderstanding.
The "pictureInPictureToggleEnabled" scalar was not being incremented when the PiP is being disabled by right-clicking the PiP button/toggle and selecting "Hide Picture-in-Picture Toggle". This issue still occurs.

Furthermore, can you provide some information on how to verify the other parts?

  1. New attributes to the close PiP event:
    closeButton - closing the PiP by the "x" button will record a PiP closed_method event with reason "close-button".
    unpipButton - closing the PiP by the "unPiP" button will record a closed_method event with reason "unpip".
    originTabClose - closing the PiP by closing the tab entirely will record a closed_method event with reason "pagehide".
    processShutdown - how do I verify this?

  2. FX_PICTURE_IN_PICTURE_WINDOW_OPEN_DURATION histogram shows results like:
    "FX_PICTURE_IN_PICTURE_WINDOW_OPEN_DURATION
    4 samples, average = 67, sum = 268

0 | 0 0%
1 |######################### 1 25%
2 |######################### 1 25%
12 |######################### 1 25%
237 |######################### 1 25%
256 | 0 0%"

It appears to behave correctly, but I am unsure how it would be verified best.

  1. New attributes to "pictureinpicture.create" event:
    subtitlesEnabled: bool - always displays as "true" - it appears to behave incorrectly!
    webVTTSubtitles: bool - shows up as "true" when opening a "pure WebVTT" video like this
    - shows up as "false" when opening other videos like from Youtube or Netflix;

I would like specific feedback on each of these implementations. Thanks!
This bug appears to be partially fixed. How would you like to proceed?

Flags: needinfo?(nbaumgardner)
  1. Sorry for the confusion on this. The closing reasons are "close-button", "unpip", "pagehide", "fullscreen", "setup-failure", "close-player-shortcut", "context-menu", or "video-el-remove". We re-used some old telemetry for this and the bug description was not updated.

  2. The results are the number of seconds a PiP window is open. For the results you pasted there were 4 videos, one video was in PiP for 1 second, another for 2, another for 12, and another for 237.

  3. That is correct. YouTube and Netflix don't use WebVTT so that bool should be false.

  4. There is already existing telemetry for recording events when the toggle is enabled and disabled. These events are recorded when the pref media.videocontrols.picture-in-picture.video-toggle.enabled is flipped. The pref is flipped when the "Hide Picture-in-Picture Toggle" is clicked, or from clicking "Enable picture-in-picture video controls" in settings, or from flipping the pref in about:config. They appear in telemetry as shown below.

Keyed Scalars
parent process

telemetry.event_counts
pictureinpicture.settings#disable#player 	2
pictureinpicture.settings#enable#player 	2 
Events
parent
timestamp	category                method     object
312625 	pictureinpicture.settings 	disable    player
344078 	pictureinpicture.settings 	enable     player
378844 	pictureinpicture.settings 	disable    player
385750 	pictureinpicture.settings 	enable     player 
Flags: needinfo?(nbaumgardner)
  1. New attributes to the close PiP event:
  • close-button - closing the PiP by the "x" button will record a PiP closed_method event with reason "close-button".
  • unpip - closing the PiP by the "unPiP" button will record a closed_method event with reason "unpip".
  • pagehide - closing the PiP by closing the tab entirely will record a closed_method event with reason "pagehide".
  • fullscreen - launching PiP, then opening full screen on the original player will record a closed_method event with reason "fullscreen".
  • context-menu - launching PiP, then closing it using context menu on the original tab will record a closed_method event with reason "context-menu".
  • setup-failure - how do I verify this?
  • close-player-shortcut - how do I verify this?
  • setup-failure - how do I verify this?
  • video-el-remove - how do I verify this?

Are the actions for the ones I guessed above correct? How do I verify the remaining ones?

  1. The PiP duration histogram seems to be working correctly.

  2. THe WebVTT part seems to work, but the subtitlesEnabled part does not:
    subtitlesEnabled: bool - always displays as "true" - it appears to behave incorrectly!
    How should it really work?

  3. There is a scalar that gets incremented ONLY when flipping the PiP preference to ON or OFF from the browser's preferences. It does not increment when disabling PiP from the context menu. This scalar appears in about:telemetry as pictureInPictureToggleEnabled under "parent processes" under "Keyed Scalars".

How should the "pictureInPictureToggleEnabled" behave?
Note that I am not reffering to "pictureinpicture.settings#disable#player" or "pictureinpicture.settings#enable#player" from under "telemetry.event_counts" but to "pictureInPictureToggleEnabled" from under the browser.ui.interaction.preferences_paneGeneral!

Reopening this enhancement as it is not completely fixed. Please let me know if you prefer another bug on the remaining issues.

Flags: needinfo?(nbaumgardner)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
  1. setup-failure: This occurs when the original video element goes away before we have had time to set up the PiP window.
    close-player-short: ctrl + shift + ] on Windows. cmd or ctrl + option + shift + ] on Mac.
    video-el-remove: This only occurs when no other reason is given. We always give another reason so it shouldn't ever be recorded unless there is an error.

  2. subtitlesEnabled is shown as true when media.videocontrols.picture-in-picture.display-text-tracks.enabled is true.

  3. The bug description only talks about pictureinpicture.settings.disable and pictureinpicture.settings.disable is recorded when "Hide Picture-in-Picture toggle” is clicked from the PiP context menu and the other places the toggle can be disabled. It behaves how we want. This bug didn't change anything with "pictureInPictureToggleEnabled" because we already have pictureinpicture.settings.enable and pictureinpicture.settings.disable that do what we want.

Flags: needinfo?(nbaumgardner)

I have also verified the "player-shortcut" attribute of the PiP close event. The setup-failure and video-el-remove attributes can't be verified since we don't know how to.

I've also verified the "ccEnabled" attribute of the PiP create event which appears as false when media.videocontrols.picture-in-picture.display-text-tracks.enabled is false and vice-versa.

Relating to the issue with the scalar that isn't incremented correctly, I'll assume it's not necessary and so, won't fix it.

Based on the above, I'm closing this bug as verified in Nightly v100.0a1 from 2022-03-16. Ania, can you confirm this is acceptable?

Flags: needinfo?(amininkova)

If yes, please set it back to "fixed" so I can officially verify it.

Relating to the issue with the scalar that isn't incremented correctly, I'll assume it's not necessary and so, won't fix it.

Thank you, Daniel, I confirm. We don't track this scalar or use it in any analytics, so it's a non-issue.

Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Flags: needinfo?(amininkova)
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Whiteboard: [fidefe-MR1-2022]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: