Closed Bug 884821 Opened 6 years ago Closed 6 years ago

Can't reinspect objects in Scratchpad

Categories

(DevTools :: Scratchpad, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 24

People

(Reporter: vporof, Assigned: bbenvie)

Details

Attachments

(1 file, 3 obsolete files)

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.
Assignee: nobody → bbenvie
Attached patch sidebar bugfix (obsolete) — Splinter Review
Bugs that are fixed solely by deleting code are fun.
Attachment #764863 - Flags: review?(vporof)
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?
Attachment #764863 - Flags: review?(vporof) → feedback+
And apart from buts you should also talk with paul about bugs.
(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.
Attached file better sidebar bugfix (obsolete) —
* Use the ToolSidebar's hide/show mechanism instead of the splitter
* Add tests that confirm the behavior of showing/hiding the inspection sidebar
Attachment #764863 - Attachment is obsolete: true
Attachment #764937 - Flags: review?(vporof)
Attached patch better sidebar bugfix (obsolete) — Splinter Review
Make it a patch <:|
Attachment #764937 - Attachment is obsolete: true
Attachment #764937 - Flags: review?(vporof)
Attachment #764940 - Flags: review?(vporof)
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.
Attachment #764940 - Flags: review?(vporof) → review+
Attached patch finalSplinter Review
Correcto!
Attachment #764940 - Attachment is obsolete: true
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/1663fa5bc540
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1663fa5bc540
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 24
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.