Closed Bug 1630228 Opened 1 year ago Closed 1 year ago

Experiment disabling F12 by default

Categories

(DevTools :: General, task, P3)

task

Tracking

(firefox77 fixed)

RESOLVED FIXED
Firefox 77
Tracking Status
firefox77 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

Attachments

(8 files, 1 obsolete file)

Experiment designed to disable F12 until users have opened the toolbox once.

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

Depends on D71035

Gijs: Hi!

We want to disable the F12 shortcut for DevTools in an experiment, since it is responsible for most of the accidental usage of DevTools.
I was asked to evaluate how complex it would be to add a small "doorhanger" notification when users press F12. The idea is to have a very simple readonly message "If you want to open DevTools, activate it via the Web Developer menu". (will add screenshot in next comment)

I tried to look at how other doorhangers were displayed and came up with the following diff: https://phabricator.services.mozilla.com/D71036
Basically I am including a new inc.xhml + ftl in browser.xhtml and I try to trigger it from DevTools. I'm missing most of the styling for now, but before I dive deeper, I'd like to get an overall feeling from someone working on the Firefox UI. Does the idea of adding a doorhanger for an experiment seem reasonable to you? And if yes, should I be careful about hidden complexity or does the skeleton of my patch look more or less on track?

(hopefully you're the good person to ask, otherwise, maybe you would know who to ping?)

Thanks!

Flags: needinfo?(gijskruitbosch+bugs)
Attached image image.png

Very sorry about the slowness here. In future, please do feel free to ping me on slack/matrix, I lost track of this needinfo for a bit.

(In reply to Julian Descottes [:jdescottes] from comment #4)

We want to disable the F12 shortcut for DevTools in an experiment, since it is responsible for most of the accidental usage of DevTools.

I'm a little surprised; this is the shortcut IE/Edge evangelize on, right? If it's responsible for most accidental usage, do we know how common intentional usage of the shortcut is?

I was asked to evaluate how complex it would be to add a small "doorhanger" notification when users press F12. The idea is to have a very simple readonly message "If you want to open DevTools, activate it via the Web Developer menu". (will add screenshot in next comment)

I tried to look at how other doorhangers were displayed and came up with the following diff: https://phabricator.services.mozilla.com/D71036
Basically I am including a new inc.xhml + ftl in browser.xhtml and I try to trigger it from DevTools. I'm missing most of the styling for now, but before I dive deeper, I'd like to get an overall feeling from someone working on the Firefox UI. Does the idea of adding a doorhanger for an experiment seem reasonable to you? And if yes, should I be careful about hidden complexity or does the skeleton of my patch look more or less on track?

It's a bit overly complex. :-) I think I saw similar confusion in the all-tabs view and forgot to chase it, so I'll do that after responding here...

I think here you only really need a panel, not a panelview, because you're not going to create sliding subviews or anything like that. You shouldn't need to use PanelMultiView. You also don't need the hidden attribute, nor do you need to set that when the popup hides. If you want to hide the popup, you can call the hidePopup method on the panel. (The hidden attribute on panels is a leftover optimization from XBL days. It really needs to be removed for all the panels that currently deal with it, but that's a separate discussion...)

You could look at the notification shown after use of the forget/panic button - https://searchfox.org/mozilla-central/rev/aec63591821712236a522f7f55116f582ed7c920/browser/components/customizableui/content/panelUI.inc.xhtml#74-90 . You just need a panel and some markup in the panel to make it look the way you want. Then when you want to show the panel, simply call openPopup, like the code at https://searchfox.org/mozilla-central/rev/aec63591821712236a522f7f55116f582ed7c920/browser/base/content/browser.js#8780-8807 - which does some extra stuff to make clicking on the panel / keypressing while the panel is up make the panel disappear, as well as having a timeout to hide it after a few seconds.

In terms of the ftl file, I'd find another topical ftl file rather than introducing a new one just for this message, and call MozXULElement.insertFTLIfNeeded with the path to the ftl file when you show the popup, if the file is not already included in browser.xhtml .

Does that help?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(jdescottes)

Thanks a lot for the pointers Gijs! I will apply them next week, but I wanted to provide some answers to your question below

I'm a little surprised; this is the shortcut IE/Edge evangelize on, right? If it's responsible for most accidental usage, do we know how common intentional usage of the shortcut is?

We have measured the accidental usage in various experiments, and the results are not always consistent. But for the study I followed closely, the results were:

  • 2/3 of DevTools opening were accidental
  • 75% of accidental open are coming from F12
  • among non-accidental users, 1/3 use F12

This leads to think that the accidental usage of F12 is 5 times more important than the intentional one.
Keeping in mind it's our only shortcut which is not using modifiers.

Flags: needinfo?(jdescottes)

I updated https://phabricator.services.mozilla.com/D71036 with your suggestions, it looks much simpler!
Waiting for inputs from Harald about the styling of the notification.

In terms of the ftl file, I'd find another topical ftl file rather than introducing a new one just for this message, and call MozXULElement.insertFTLIfNeeded with the path to the ftl file when you show the popup, if the file is not already included in browser.xhtml .

I put it in browser.ftl for now for the sake of simplicity, as we don't really have a devtools' ftl for popups, but I can change that later.

Attached file bug1630228.datareview.txt (obsolete) —
Attachment #9142739 - Flags: data-review?(tdsmith)
Comment on attachment 9142739 [details]
bug1630228.datareview.txt

1) Is there or will there be **documentation** that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, in Events.yaml and the probe dictionary.

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

Yes, the Firefox telemetry opt-out.

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

Yes, Harald.

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 2, interaction data.

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?

jdescottes and Harald are responsible for renewing the temporary collection before it expires.

9) Does the data collection use a third-party collection tool?

No.
Attachment #9142739 - Flags: data-review?(tdsmith) → data-review+
Attached image image.png

For reference - this ETP message is where we currently use a doorhanger for a simple notification.

Updated per comments from tdsmith on https://phabricator.services.mozilla.com/D71988

Attachment #9142739 - Attachment is obsolete: true
Attachment #9144127 - Flags: data-review?(tdsmith)
Comment on attachment 9144127 [details]
bug1630228.datareview.2.txt

Thanks for the changes! Responses for the prior review still apply.
Attachment #9144127 - Flags: data-review?(tdsmith) → data-review+
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae64b5ffc3b5
Basic implementation to disable F12 until toolbox opens r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/f9e6d07f3a90
Show a doorhanger when using F12 r=fluent-reviewers,nchevobbe,Gijs,victoria
https://hg.mozilla.org/integration/autoland/rev/11f6beab6bdf
Enable F12 for all users with selfxss pref > 0 r=Harald,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/8a9c30923847
Add telemetry events for OFF 12 devtools experiment r=nchevobbe,janerik
https://hg.mozilla.org/integration/autoland/rev/92172b9cef6b
Add tests for the F12 disabled experiment r=nchevobbe

Sorry about that!

(In reply to Narcis Beleuzu [:NarcisB] from comment #17)

Backed out for bc failures on browser_startup_images.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/bc7658646927718a3c7b7174cb0e6df7130afec3
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299883784&repo=autoland&lineNumber=1048

Looks like this is what the hidden attribute on the <panel> element is for after all.
Will add the attribute and re-request review.

Also dt failure on browser_toolbox_telemetry_open_event.js: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299882920&repo=autoland&lineNumber=3412

Forgot to remove a reference to an extra_key, fixing this.

Flags: needinfo?(jdescottes)
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8f6dc8f06ce
Basic implementation to disable F12 until toolbox opens r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/91d89a77be32
Show a doorhanger when using F12 r=fluent-reviewers,nchevobbe,Gijs,victoria
https://hg.mozilla.org/integration/autoland/rev/df2a2b6c118e
Enable F12 for all users with selfxss pref > 0 r=Harald,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/b2f7e6085ee8
Add telemetry events for OFF 12 devtools experiment r=nchevobbe,janerik
https://hg.mozilla.org/integration/autoland/rev/7ecc2806071e
Add tests for the F12 disabled experiment r=nchevobbe
Regressions: 1634195

Note: if you want to test this locally on Nightly or local builds, make sure you flip:

  • devtools.experiment.f12.shortcut_disabled to true
  • devtools.selfxss.count to 0

The first preference enables the experiment, and the second preference is to flag the profile as a "new devtools user".

Depends on: 1636591
See Also: → 1643427
You need to log in before you can comment on or make changes to this bug.