884821 - Can't reinspect objects in Scratchpad

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect)

Description

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

STR:

1. Open scratchpad
2. Type "window" without quotes
3. Ctrl/Cmd+I
4. Escape
5. Ctrl/Cmd+I

The sidebar containing the variables view doesn't reopen. No errors in the Browser Console.

Comment 1

[Brandon Benvie [:benvie]]

Attachment: sidebar bugfix

Bugs that are fixed solely by deleting code are fun.

Comment 2

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

Comment on attachment 764863 [details] [diff] [review]
sidebar bugfix

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

Um.. This looks alright but I don't get it :)

Was there a but in the sidebar's hide/show methods? I think paul wrote that code, you should have a chat with him.
Also, how about writing a test for this so we don't regress it in the future?

Comment 3

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

And apart from buts you should also talk with paul about bugs.

Comment 4

[Brandon Benvie [:benvie]]

(In reply to Victor Porof [:vp] from comment #2)
> Um.. This looks alright but I don't get it :)

I thought I did, but it appears I accidentally got a working solution rather than being correct in my understanding of why it was broken.


> Was there a but in the sidebar's hide/show methods? I think paul wrote that
> code, you should have a chat with him.

I don't believe so. Upon closer examination, if EITHER the tabbox is hidden/shown OR the splitter is collapsed/uncollapsed, it works fine. But if BOTH are done, then it never reopens. So I'm chalking it up to the some interaction between the XUL element states of the splitter and the tabbox.


> Also, how about writing a test for this so we don't regress it in the future?

Good call. Now that I correctly understand what happened I'll add a test.

Comment 5

[Brandon Benvie [:benvie]]

Attachment: better sidebar bugfix

* Use the ToolSidebar's hide/show mechanism instead of the splitter
* Add tests that confirm the behavior of showing/hiding the inspection sidebar

Comment 6

[Brandon Benvie [:benvie]]

Attachment: better sidebar bugfix

Make it a patch <:|

Comment 7

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

Comment on attachment 764940 [details] [diff] [review]
better sidebar bugfix

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

::: browser/devtools/scratchpad/scratchpad.xul
@@ +287,5 @@
>  
>  <notificationbox id="scratchpad-notificationbox" flex="1">
>    <hbox flex="1">
>      <vbox id="scratchpad-editor" flex="1"/>
> +    <splitter class="devtools-side-splitter" collapse="after"/>

I think you can also remove the collapse attribute.

Comment 8

[Brandon Benvie [:benvie]]

Attachment: final

Correcto!

Comment 9

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

https://hg.mozilla.org/integration/fx-team/rev/1663fa5bc540

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/1663fa5bc540