Closed
Bug 1371842
Opened 7 years ago
Closed 7 years ago
Update Debugger frontend (6/9/2017)
Categories
(DevTools :: Debugger, defect)
DevTools
Debugger
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jlast, Assigned: jlast)
References
Details
Attachments
(1 file, 2 obsolete files)
791.81 KB,
patch
|
jdescottes
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8876311 -
Flags: review?(jdescottes)
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8876311 -
Attachment is obsolete: true
Attachment #8876311 -
Flags: review?(jdescottes)
Attachment #8876314 -
Flags: review?(jdescottes)
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=407e8dda4a3836c421a618b5a6ce21ca4023a5aa
Assignee | ||
Comment 4•7 years ago
|
||
By the way, this patch has a change to attach-thread, shared/client/main to enable the faster stepping.
Comment 5•7 years ago
|
||
Comment on attachment 8876314 [details] [diff] [review] 6-9-1.patch Review of attachment 8876314 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure what's the best solution for this kind of patches. On one hand I'd like to have only files coming from GitHub in the changeset (and move the changes from attach-thread & main.js to another one). On the other hand if it gets backed out, it's easier to deal with a single changeset. Same functional regression with the search: - start searching with CMD+F - type something - press ESCAPE - press CMD+F => Search input has been cleared. With the current version of the new debugger the search input still contains the previous value. Mentioning it here since I'm not sure if it's intended. I'll let you set this one as checkin-needed. We're reaching the end of the code freeze, I prefer to let sheriffs decide how much they want to merge. ::: devtools/client/framework/attach-thread.js @@ +46,5 @@ > // client-side, so the server should not do it. It also does not support > // blackboxing yet. > let useSourceMaps = false; > let autoBlackBox = false; > + let ignoreFrameEnvironment = false; nits: ignoreFrameEnvironment = newDebuggerEnabled. The comment above looks a bit outdated too : "It also does not support blackboxing yet."
Attachment #8876314 -
Flags: review?(jdescottes) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 6•7 years ago
|
||
thanks julian. Hopefully this gets merged because the new UI is not on by default, so the risk is low. But if it doesn't that's okay too. I think you're right that the previous version cleared the search results, but it's not clear what the right behavior should be. Both chrome devtools and atom keeps the cmd+f space in this scenario. I'm sure others clear it. Good point on the comment. Updated.
Assignee | ||
Comment 7•7 years ago
|
||
Attachment #8876314 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8876360 -
Flags: review+
Comment 8•7 years ago
|
||
Comment on attachment 8876360 [details] [diff] [review] 6-9-2.patch Review of attachment 8876360 [details] [diff] [review]: ----------------------------------------------------------------- One tiny nit: sha1/tag is missing from the commit message.
Updated•7 years ago
|
Assignee: nobody → jlaster
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7583224b1a42 Update Debugger frontend (6/9/2017). r=jdescottes
Keywords: checkin-needed
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7583224b1a42
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•