Closed Bug 895543 Opened 7 years ago Closed 6 years ago

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

Categories

(DevTools :: Debugger, defect)

25 Branch
x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 25

People

(Reporter: fitzgen, Assigned: fitzgen)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

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.
Depends on: 877686
Assignee: nobody → nfitzgerald
Attachment #783414 - Flags: review?(rcampbell)
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
This patch includes the images.
Attachment #783414 - Attachment is obsolete: true
Attachment #783414 - Flags: review?(rcampbell)
Attachment #783419 - Flags: review?(rcampbell)
Attachment #783413 - Flags: review?(rcampbell) → review+
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.
Attachment #783419 - Flags: review?(rcampbell) → review-
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.
Attachment #783419 - Flags: review- → review+
Uploading the version of the patch that landed in fx-team for posterity.
Attachment #783419 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/0ded78d3e616
https://hg.mozilla.org/mozilla-central/rev/99e620258012
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 25
Depends on: 901296
You weren't kidding when you said "big". IMHO the font size (and thus the message and button) are a bit too big.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.