1093349 - Add pretty printing and blackboxing traits and hide buttons conditionally

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[James Long (:jlongster)]

This is for Fever Dream, which right now doesn't support pretty printing or blackboxing. We want to simply remove those buttons in the debugger if those actions aren't supported, and we specify that by adding some traits.

Comment 1

[James Long (:jlongster)]

Attachment: 1093349.patch

Comment 2

[James Long (:jlongster)]

Attachment: 1093349.patch

Comment 3

[James Long (:jlongster)]

Comment on attachment 8516317 [details] [diff] [review]
1093349.patch

Not sure who I should ask to review this; past could you take a quick look?

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8516317 [details] [diff] [review]
1093349.patch

Review of attachment 8516317 [details] [diff] [review]:
-----------------------------------------------------------------

Could you also add a simple test for this? Something like manually starting a debugger server, forcing the new traits to true, opening the toolbox and then verifying that the buttons are hidden.

::: browser/devtools/debugger/debugger-view.js
@@ +72,5 @@
>    },
>  
> +  connect: function() {
> +    if (gClient.mainRoot.traits.noPrettyPrinting) {
> +      this.Sources.hidePrettyPrinting();

My only concern is that you are now introducing a dependency to the remote debugging protocol in DebuggerView. We've tried to only make the controller protocol-aware, so if you could move this function there, this would be perfect!

Comment 5

[James Long (:jlongster)]

(In reply to Panos Astithas [:past] (overloaded, please needinfo) from comment #4)
> 
> My only concern is that you are now introducing a dependency to the remote
> debugging protocol in DebuggerView. We've tried to only make the controller
> protocol-aware, so if you could move this function there, this would be
> perfect!

Great, already done. Just need to write the test, will do that first thing tomorrow.

Comment 6

[James Long (:jlongster)]

Attachment: 1093349.patch

Added a test and made the controller hide the buttons

Comment 7

[Carsten Book [:Tomcat]]

Hi, can we get a try link for this changes ? :) 

Thanks!

Comment 8

[James Long (:jlongster)]

Sure thing. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c7b0ddfba982 This only touches the debugger

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/85786620f4af

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/85786620f4af