Update Debugger frontend (7/19/2017).

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Developer Tools: Debugger
P3
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: jlast, Assigned: jlast)

Tracking

unspecified
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment, 6 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

5 months ago
Created attachment 8888079 [details] [diff] [review]
7-19.patch
Attachment #8888079 - Flags: review?(jdescottes)
Assignee: nobody → jlaster
Status: NEW → ASSIGNED
Priority: -- → P3
Comment on attachment 8888079 [details] [diff] [review]
7-19.patch

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

Project search yay! Some weird things I noticed while testing:

Re. project search
1. Clicking in [Cmd+P to Search] in the left sidebar opens the project search instead of the file search
2. Project search has no scrollbar so you can't see all results
3. Could not open the project search with the dedicated keyboard shortcut?
4. Can't navigate from project search to actual results.

I have seen other bugs, but I am not sure if project search is actually ready for release? Should make sure users can't start it if it's not ready.

There is a little UI regression which shifts the list of sources when selecting one (will attach a GIF).
Would be nice to fix as it feels buggy and I don't know if we will have another bundle release before the end of the 56 cycle.

We no longer shorten the filenames in the callstack but instead we get a vertical scrollbar for the panel. Is this expected, I'm not sure this is nice behavior? (will attach a screenshot). 

Not a regression, but breakpoints disappear when switching from vertical to horizontal layout. 

Clearing review flag for now.

::: devtools/client/locales/en-US/debugger.properties
@@ +136,5 @@
>  # search for searching all the source files the debugger has seen.
>  sources.search.alt.key=CmdOrCtrl+O
>  
> +# LOCALIZATION NOTE (projectTextSearch.key): A key shortcut to open the
> +# full projct text search for searching all the files the debugger has seen.

nit: projct -> project
Attachment #8888079 - Flags: review?(jdescottes)
Created attachment 8888211 [details]
breakpoints_disappear.gif

illustrate disappearing breakpoints (not a regression)
Created attachment 8888212 [details]
selecting_source_shifts_list.gif

illustrate list of sources shifting on selection (ui regression)
Created attachment 8888213 [details]
right_sidebar_vertical_scrollbar.png

illustrate vertical scrollbar (linked to callstack change?)
(Assignee)

Comment 6

5 months ago
Created attachment 8888383 [details] [diff] [review]
7-19-2.patch
Attachment #8888079 - Attachment is obsolete: true
Attachment #8888211 - Attachment is obsolete: true
Attachment #8888212 - Attachment is obsolete: true
Attachment #8888213 - Attachment is obsolete: true
Attachment #8888383 - Flags: review?(jdescottes)
(Assignee)

Comment 7

5 months ago
Created attachment 8888475 [details] [diff] [review]
7-19-3.patch
Attachment #8888383 - Attachment is obsolete: true
Attachment #8888383 - Flags: review?(jdescottes)
Attachment #8888475 - Flags: review?(jdescottes)
(Assignee)

Comment 8

5 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6323e535b88d604c962b16807b50bc513cdd2779
Comment on attachment 8888475 [details] [diff] [review]
7-19-3.patch

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

A small thing that needs to be backported to GitHub otherwise this looks good to me.
Did you get UX feedback on the horizontal scrollbar? I'm still not a fan :) but won't block the review on that.

::: devtools/client/debugger/new/test/mochitest/.eslintrc
@@ +25,5 @@
>      "requestLongerTimeout": false,
>      "SimpleTest": false,
>      "SpecialPowers": false,
>      "TestUtils": false,
> +    "thisTestLeaksUncaughtRejectionsAndShouldBeFixed": false,

This was removed by https://hg.mozilla.org/mozilla-central/rev/37651b6bd99c recently. We probably want to keep it as is.
Attachment #8888475 - Flags: review?(jdescottes) → review+
(Assignee)

Comment 10

5 months ago
Created attachment 8888611 [details] [diff] [review]
7-19-4.patch
Attachment #8888475 - Attachment is obsolete: true
(Assignee)

Updated

5 months ago
Keywords: checkin-needed

Comment 11

5 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f821df0f23c
Update debugger frontend (v0.8.0). r=jdescottes
Keywords: checkin-needed
Blocks: 1383148
Blocks: 1380085
https://hg.mozilla.org/mozilla-central/rev/3f821df0f23c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.