[remote-dbg-next] Add control to enable addon debugging in new about:debugging
Categories
(DevTools :: about:debugging, enhancement, P1)
Tracking
(firefox67 verified, firefox68 verified)
People
(Reporter: jdescottes, Assigned: daisuke)
References
(Blocks 1 open bug)
Details
(Whiteboard: old-remote-debugging-ng-m3)
Attachments
(5 files, 1 obsolete file)
prototype.png
about:debugging gap With the current about:debugging, if you go to about:debugging#addons, there is a checkbox that allows to enable/disable addons debugging: [ ] Enable add-on debugging Learn more Behind the scenes, this sets a couple of preferences as detailed on https://developer.mozilla.org/en-US/docs/Tools/about:debugging#Enabling_add-on_debugging This feature is missing from the new about:debugging and should be implemented.
|
Reporter | |
Comment 1•6 years ago
|
||
filter on remote-debugging-next-move-m3-to-m2
|
|
Reporter | |
Comment 2•6 years ago
|
||
filter on remote-debugging-next-move-m3-to-m2
|
|
Reporter | |
Comment 3•6 years ago
|
||
filter on remote-debugging-next-move-m3-to-m2
|
|
Reporter | |
Comment 5•5 years ago
|
||
The Inspect buttons should also be disabled when this preference is not set.
|
|
Reporter | |
Comment 6•5 years ago
|
||
If tests are added to cover this, we can close Bug 1500385 as Duplicate.
|
Assignee | |
Comment 8•5 years ago
|
||
This checkbox is valid for addons on remote runtimes such as Android as well?
As far as I investigate, we can debug any addons on remote even devtools.chrome.enabled and devtools.debugger.remote-enabled config on remote are off.
If the configs are valid for remote runtime as well, it seems we should implement also a mechanism which accepts/rejects the debugging on remote runtime by the configs, I think.
|
|
Reporter | |
Comment 9•5 years ago
|
||
Ah that's interesting.
devtools.chrome.enabled
It looks like devtools.chrome.enabled is mostly checked on the client side. The only server side usage of this preference is:
So for the runtime (=server) turning on this pref will only impact the storage inspector.
For local addon debugging, it is checked on the client side to open the browser toolbox, which is also what we use for local addons. But it looks like we can skip it for remote runtimes.
devtools.debugger.remote-enabled
I have not tested, but I assume that devtools.debugger.remote-enabled is always true when we are debugging a remote device? The purpose of the pref is to enable remote debugging, so it would make sense? https://searchfox.org/mozilla-central/search?q=devtools.debugger.remote-enabled&case=true®exp=false&path=mobile
This control is only needed because we use the browser toolbox at the moment to debug addons. We should show it only for This-Firefox. For the remote runtimes, the chrome enabled pref is client side only so we don't care here. And if you are connected to the remote runtime, then it means devtools.debugger.remote-enabled is already true. And no need to introduce another way to enable/disable addon debugging in remote runtimes.
This information will be useful to find the good wording we should use for this checkbox.
|
|
Assignee | |
Comment 10•5 years ago
|
||
Thanks Julian!
Ah, however, we can debug any targets on Android even devtools.debugger.remote-enabled is off...
(But if set devtools.remote.usb.enabled to off, rejects the debugging and closes current connection immediately..)
Okay, I'll file a bug to discuss what should do on remote runtime by devtools.debugger.remote-enabled.
Also, I'll take this bug, and implement only This-Firefox at this time.
Thanks again.
|
|
Assignee | |
Comment 11•5 years ago
|
||
|
|
Assignee | |
Comment 12•5 years ago
|
||
Depends on D18196
|
|
Assignee | |
Comment 13•5 years ago
|
||
Depends on D18198
|
|
Assignee | |
Comment 14•5 years ago
|
||
Depends on D18199
|
|
Assignee | |
Comment 15•5 years ago
|
||
Hi Matt, I need your advice!
As Julian said in D18196 at phabricator, we will use a checkbox here.
I have referred from checkboxes of photon. Although it defines the style of checkbox which assumes font-size: 13px, there are no definition for other font-size. Since the font-size of the "Load Temporary Add-on" button which will be at side of this checkbox is 15px, I thought it is better to fit to the font-size though, how should we design for the checkbox and the label?
|
||
Comment 16•5 years ago
|
||
Matt, the "learn more" link should look like a link, or like a Photon micro button? Thanks!
|
||
Comment 17•5 years ago
|
||
@Daisuke
Could you provide an image or screenshot of what it looks like? If I can see the page as a whole with the checkbox in place I can hopefully come up with some suggestions :)
@Belén
It depends where 'Learn more' will be placed. Generally if it is in the main body a link is correct, if this is in a message bar then Photon only indicates that a button should be used. I have not seen a text link within a message bar in use.
|
|
Assignee | |
Comment 18•5 years ago
|
||
Hi Matt, thank you for replying!
I attach the screenshot. If you need additional something, please let me know!
|
|
||
Comment 19•5 years ago
|
||
Thank you for the image!
The "Load Temporary Add-on" button will eventually follow the Photon Default button styling: https://design.firefox.com/photon/components/buttons.html#default-2 making the button text 13px. This might make it easier to have the checkbox label match the docs and also be 13px.
My advice would be to have the checkbox label 13px and see how it looks after we change the button to match the docs.
It might be that after we add more content to that page, it might look odd being the only 13px sentence, but we can make that decision once more styling is in place.
|
|
||
Comment 20•5 years ago
|
||
Just adding the feedback I received from Emanuela on #ux regarding checkbox label size.
Checkbox labels are NOT limited to Photon Body 10 (13px) type, you can use larger text (Body 20 for example) however the checbox size does not change.
|
|
Assignee | |
Comment 21•5 years ago
|
||
Depends on D18198
|
||
Updated•5 years ago
|
|
||
Comment 23•5 years ago
|
||
Pushed by dakatsuka@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9a1f91d90de1 Add a checkbox to enable addon debugging on this-firefox. r=jdescottes,ladybenko https://hg.mozilla.org/integration/autoland/rev/b10932876150 Re-layout for runtime controllers. r=ladybenko,jdescottes https://hg.mozilla.org/integration/autoland/rev/ef94872bfd12 Disable 'Inspect' button for extension when extension debug setting was disabled. r=jdescottes,ladybenko https://hg.mozilla.org/integration/autoland/rev/dd4aa59c6a12 Add a test for extension debug setting. r=jdescottes,ladybenko
|
||
Comment 24•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/9a1f91d90de1
https://hg.mozilla.org/mozilla-central/rev/b10932876150
https://hg.mozilla.org/mozilla-central/rev/ef94872bfd12
https://hg.mozilla.org/mozilla-central/rev/dd4aa59c6a12
|
||
Comment 25•5 years ago
|
||
Verified as fixed on Firefox Nightly 68.0a1 (2019-04-01) and Firefox 67.0b6 on Windows 10 x 64, Mac OS X 10.14 and on Ubuntu 16.04 x64.
Description
•