895543 - show a "this source is black boxed" message and big "un black box" button instead of the source editor for black boxed sources

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

After discussing the UI for black boxed sources with Victor, we decided the best way to keep people from getting confused with regards to breakpoints, sources, and black boxing is to not let them add breakpoints while black boxed. We do this by hiding the source (consistent with black boxing attitude of "I don't care about these details, hide them from me") and instead showing a message that the source is black boxed and making it really easy to un black box right there.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: v1 - part 1 - cache source clients

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: v1 - part 2 - add black boxed message

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Haven't run the new mochitest locally yet, waiting for the fix to merge into fx-team. Does work in my own experimenting however. Here's a try push:

https://tbpl.mozilla.org/?tree=Try&rev=7111d420cb4e

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: v1.1 - part 2 - add black boxed message

This patch includes the images.

Comment 5

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 783419 [details] [diff] [review]
v1.1 - part 2 - add black boxed message

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

r- because of the missing style things. Otherwise looks good.

::: browser/devtools/debugger/debugger-panes.js
@@ +25,5 @@
>    this._onConditionalPopupShowing = this._onConditionalPopupShowing.bind(this);
>    this._onConditionalPopupShown = this._onConditionalPopupShown.bind(this);
>    this._onConditionalPopupHiding = this._onConditionalPopupHiding.bind(this);
>    this._onConditionalTextboxInput = this._onConditionalTextboxInput.bind(this);
>    this._onConditionalTextboxKeyPress = this._onConditionalTextboxKeyPress.bind(this);

wow. this method has a lot of this._onSomething = this._onSomething.bind(this);.

::: browser/devtools/debugger/debugger.xul
@@ +31,5 @@
>    <commandset id="editMenuCommands"/>
>    <commandset id="sourceEditorCommands"/>
>  
>    <commandset id="debuggerCommands">
> +    <command id="unBlackBoxButton"

it's kind of an ugly name for an id, but not really seeing anything better. clearBlackBoxButton? deBlackBox... ? horrible...

@@ +32,5 @@
>    <commandset id="sourceEditorCommands"/>
>  
>    <commandset id="debuggerCommands">
> +    <command id="unBlackBoxButton"
> +             oncommand="DebuggerView.Sources._onStopBlackBoxing()"/>

blackboxing is a terrible verb. :)

everything is terrible!

ok.

::: browser/themes/linux/devtools/debugger.css
@@ +56,5 @@
>  
> +/* Black box message */
> +
> +#black-boxed-message {
> +  padding: 100px 50px;

no background-color?

::: browser/themes/windows/devtools/debugger.css
@@ +56,5 @@
>  
> +/* Black box message */
> +
> +#black-boxed-message {
> +  padding: 100px 50px;

no background color.

::: browser/themes/windows/jar.mn
@@ +212,5 @@
>          skin/classic/browser/devtools/inspect-button.png            (devtools/inspect-button.png)
>          skin/classic/browser/devtools/dropmarker.png                (devtools/dropmarker.png)
>          skin/classic/browser/devtools/layout-background-grid.png    (devtools/layout-background-grid.png)
>          skin/classic/browser/devtools/layoutview.css                (devtools/layoutview.css)
>          skin/classic/browser/devtools/debugger-collapse.png         (devtools/debugger-collapse.png)

need a pointer to this in skin/aero/browser/devtools as well.

Comment 6

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 783419 [details] [diff] [review]
v1.1 - part 2 - add black boxed message

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

r+ with the updated style things. looks good.

::: browser/devtools/debugger/debugger-panes.js
@@ +25,5 @@
>    this._onConditionalPopupShowing = this._onConditionalPopupShowing.bind(this);
>    this._onConditionalPopupShown = this._onConditionalPopupShown.bind(this);
>    this._onConditionalPopupHiding = this._onConditionalPopupHiding.bind(this);
>    this._onConditionalTextboxInput = this._onConditionalTextboxInput.bind(this);
>    this._onConditionalTextboxKeyPress = this._onConditionalTextboxKeyPress.bind(this);

wow. this method has a lot of this._onSomething = this._onSomething.bind(this);.

::: browser/devtools/debugger/debugger.xul
@@ +31,5 @@
>    <commandset id="editMenuCommands"/>
>    <commandset id="sourceEditorCommands"/>
>  
>    <commandset id="debuggerCommands">
> +    <command id="unBlackBoxButton"

it's kind of an ugly name for an id, but not really seeing anything better. clearBlackBoxButton? deBlackBox... ? horrible...

@@ +32,5 @@
>    <commandset id="sourceEditorCommands"/>
>  
>    <commandset id="debuggerCommands">
> +    <command id="unBlackBoxButton"
> +             oncommand="DebuggerView.Sources._onStopBlackBoxing()"/>

blackboxing is a terrible verb. :)

everything is terrible!

ok.

::: browser/themes/linux/devtools/debugger.css
@@ +56,5 @@
>  
> +/* Black box message */
> +
> +#black-boxed-message {
> +  padding: 100px 50px;

no background-color?

::: browser/themes/windows/devtools/debugger.css
@@ +56,5 @@
>  
> +/* Black box message */
> +
> +#black-boxed-message {
> +  padding: 100px 50px;

no background color.

::: browser/themes/windows/jar.mn
@@ +212,5 @@
>          skin/classic/browser/devtools/inspect-button.png            (devtools/inspect-button.png)
>          skin/classic/browser/devtools/dropmarker.png                (devtools/dropmarker.png)
>          skin/classic/browser/devtools/layout-background-grid.png    (devtools/layout-background-grid.png)
>          skin/classic/browser/devtools/layoutview.css                (devtools/layoutview.css)
>          skin/classic/browser/devtools/debugger-collapse.png         (devtools/debugger-collapse.png)

need a pointer to this in skin/aero/browser/devtools as well.

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://hg.mozilla.org/integration/fx-team/rev/0ded78d3e616
https://hg.mozilla.org/integration/fx-team/rev/99e620258012

Comment 8

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: v1.2 - part 2 - add black boxed message

Uploading the version of the patch that landed in fx-team for posterity.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0ded78d3e616
https://hg.mozilla.org/mozilla-central/rev/99e620258012

Comment 10

[Victor Porof [:vporof][:vp]]

You weren't kidding when you said "big". IMHO the font size (and thus the message and button) are a bit too big.