Have a classic vertical display of stack frames persistently available in the debugger UI

RESOLVED FIXED in Firefox 28

Status

defect
P2
normal
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: vporof, Assigned: vporof)

Tracking

unspecified
Firefox 28
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Assignee

Description

6 years ago
No description provided.
Assignee

Updated

6 years ago
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Priority: -- → P2
Duplicate of this bug: 911922
Assignee

Updated

6 years ago
See Also: → 918920
Assignee

Updated

6 years ago
Blocks: 926538
Assignee

Updated

6 years ago
Blocks: 850558
Assignee

Comment 2

6 years ago
Posted patch dbg-stack.patch (obsolete) — Splinter Review
Works. Needs a small test to verify that the contents are mirrored from the breadcrumbs view.
Assignee

Comment 3

6 years ago
Panos, do you think this list is better suited on the left or right of the debugger?

Here are a few thoughts (in LTR terminology):

Right:
Pros:
* familiarity with other tools in other browsers
* no need to create a new tabbox (not an argument after bug 929349)
* feels somewhat balanced: all "debugging instruments" are on the right, and "debuggee sources" on the left
Cons:
* users might be required to switch frequently between the variables tab and the call stack tab while debugging (might be alleviated by bug 725235, but not entirely)

Left:
Pros:
* ability to keep the call stack tab selected for longer, since picking a source might be infrequent (usually) and stepping already automatically jumps to the source for the topmost frame; can also pick sources via the search box
* "things that select sources" are on the left (not an argument after bug 918416)
Cons:
* might look out of place after bug 929349 (maybe we should find a better place for the traces list)
* different from all the other developer tools out there
Meh:
* resembles our debugger back in the good ol' days

I'm leaning towards placing it on the left (although this patch adds it on the right). I'm interesting in hearing your opinion.
Flags: needinfo?(past)
I agree that the left side seems like the better choice, primarily due to the need to have it visible at the same time as the variables view.
Flags: needinfo?(past)
Assignee

Comment 5

6 years ago
Posted patch v2 (obsolete) — Splinter Review
Moved call stack on the left.
Attachment #8337754 - Attachment is obsolete: true
Assignee

Comment 6

6 years ago
Posted patch v3 (obsolete) — Splinter Review
Added tests.
Attachment #8340345 - Attachment is obsolete: true
Attachment #8342924 - Flags: review?(rcampbell)
Assignee

Comment 7

6 years ago
It'd be great if we could land this before the Aurora uplift.
Assignee

Comment 8

6 years ago
Posted image looks like dis (obsolete) —
Assignee

Comment 10

6 years ago
Posted patch v4Splinter Review
Rebased.
Attachment #8342924 - Attachment is obsolete: true
Attachment #8342924 - Flags: review?(rcampbell)
Attachment #8343713 - Flags: review?(rcampbell)
Comment on attachment 8343713 [details] [diff] [review]
v4

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

this looks fine to me. As discussed in irc, I'd like to see this without the icon in the stack list using a more visible background color for selected frame.

Pbrosset also noticed that we're showing the grippy in the sidebar splitter. If we could get a fix for that too, it would be nice (or a followup bug).

::: browser/devtools/debugger/debugger.xul
@@ +325,5 @@
> +              class="devtools-sidebar-tabs">
> +        <tabs>
> +          <tab id="sources-tab" label="&debuggerUI.tabs.sources;"/>
> +          <tab id="callstack-tab" label="&debuggerUI.tabs.callstack;"/>
> +        </tabs>

can we make the <tabs> hidden by default? Only show if there are frames to display? Worried about eroding the vertical space for our source list.

::: browser/themes/linux/devtools/netmonitor.css
@@ +313,5 @@
>  /* SideMenuWidget */
>  
> +.side-menu-widget-container {
> +  box-shadow: none !important;
> +}

opportunistic fix?
Attachment #8343713 - Flags: ui-review?(dhenein)
Attachment #8343713 - Flags: review?(rcampbell)
Attachment #8343713 - Flags: review+
Assignee

Comment 12

6 years ago
(In reply to Rob Campbell [:rc] (:robcee) from comment #11)
> Comment on attachment 8343713 [details] [diff] [review]
> v4
> 
> Review of attachment 8343713 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> this looks fine to me. As discussed in irc, I'd like to see this without the
> icon in the stack list using a more visible background color for selected
> frame.

Thank you. Will do.

> Pbrosset also noticed that we're showing the grippy in the sidebar splitter.
> If we could get a fix for that too, it would be nice (or a followup bug).
> 

It's not caused by this patch, and it happens with all panes, so it's a recent regression from somewhere else. I'll file a bug.

> ::: browser/devtools/debugger/debugger.xul
> @@ +325,5 @@
> > +              class="devtools-sidebar-tabs">
> > +        <tabs>
> > +          <tab id="sources-tab" label="&debuggerUI.tabs.sources;"/>
> > +          <tab id="callstack-tab" label="&debuggerUI.tabs.callstack;"/>
> > +        </tabs>
> 
> can we make the <tabs> hidden by default? Only show if there are frames to
> display? Worried about eroding the vertical space for our source list.
> 

I guess we could. I'll try to cook something simple for this patch. If it proves to be harder than expected, I'll do it in a followup.

> ::: browser/themes/linux/devtools/netmonitor.css
> @@ +313,5 @@
> >  /* SideMenuWidget */
> >  
> > +.side-menu-widget-container {
> > +  box-shadow: none !important;
> > +}
> 
> opportunistic fix?

Nope. It's there because of the new tabbox.
Assignee

Comment 13

6 years ago
(In reply to Victor Porof [:vp] from comment #12)
> 
> > Pbrosset also noticed that we're showing the grippy in the sidebar splitter.
> > If we could get a fix for that too, it would be nice (or a followup bug).
> > 
> 
> It's not caused by this patch, and it happens with all panes, so it's a
> recent regression from somewhere else. I'll file a bug.
> 

Bug 947258.

> > ::: browser/devtools/debugger/debugger.xul
> > @@ +325,5 @@
> > > +              class="devtools-sidebar-tabs">
> > > +        <tabs>
> > > +          <tab id="sources-tab" label="&debuggerUI.tabs.sources;"/>
> > > +          <tab id="callstack-tab" label="&debuggerUI.tabs.callstack;"/>
> > > +        </tabs>
> > 
> > can we make the <tabs> hidden by default? Only show if there are frames to
> > display? Worried about eroding the vertical space for our source list.
> > 
> 
> I guess we could. I'll try to cook something simple for this patch. If it
> proves to be harder than expected, I'll do it in a followup.

This will be even weirder after bug 929349. Filed bug 947257
Assignee

Comment 14

6 years ago
Comment on attachment 8343713 [details] [diff] [review]
v4

Talked to darring on IRC.
Attachment #8343713 - Flags: ui-review?(dhenein)
Assignee

Comment 15

6 years ago
s/darring/darrin :)
Assignee

Updated

6 years ago
Attachment #8342979 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/26167138c266
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Blocks: 976320
Duplicate of this bug: 883213

Updated

Last year
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.