Closed Bug 1423158 Opened 7 years ago Closed 7 years ago

Update Debugger Frontend (12-5)

Categories

(DevTools :: Debugger, enhancement, P3)

Product:
DevTools
Component:
Debugger
Version:
58 Branch
Type:
enhancement

Tracking

(firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: yulia, Assigned: yulia)

References

Details

Attachments

(3 files, 10 obsolete files)

Bug 1423158 - Update Debugger Frontend (12-5)
(deleted), text/x-review-board-request
jdescottes
: review+
Details
debugger_close_open_bugs.gif
(deleted), image/gif
Details
test
(deleted), patch
Details | Diff | Splinter Review
Update Debugger Frontend December 5th, 2017
Attached patch Update Debugger Frontend 12-5-17 (obsolete) (deleted) — — Splinter Review
Attachment #8934529 - Flags: review?(jlaster)
Attachment #8934529 - Flags: review?(jdescottes)
Attached patch Bug 1423158 - Update Debugger Frontend (12-5) (obsolete) (deleted) — — Splinter Review
Assignee: nobody → ystartsev
Attachment #8934529 - Attachment is obsolete: true
Attachment #8934529 - Flags: review?(jlaster)
Attachment #8934529 - Flags: review?(jdescottes)
Attachment #8934531 - Flags: review?(jlaster)
Attachment #8934531 - Flags: review?(jdescottes)
Blocks: debugger-bundle-updates
Severity: normal → enhancement
Status: NEW → ASSIGNED
Priority: -- → P3
Attachment #8934531 - Attachment is obsolete: true
Attachment #8934531 - Flags: review?(jlaster)
Attachment #8934531 - Flags: review?(jdescottes)
Comment on attachment 8934533 [details] Bug 1423158 - Update Debugger Frontend (12-5) https://reviewboard.mozilla.org/r/205440/#review211046 Code wise this looks OK, but the changeset is missing the new images as well as the new test files.
Attachment #8934533 - Flags: review?(jdescottes)
Attached patch Bug 1423158 - Update Debugger Frontend (12-5) (obsolete) (deleted) — — Splinter Review
Attachment #8934533 - Attachment is obsolete: true
Attachment #8934564 - Flags: review?(jlaster)
Attachment #8934564 - Flags: review?(jdescottes)
Try failures worth looking into: - devtools/client/framework/test/browser_source_map-01.js and devtools/client/framework/test/browser_source_map-absolute.js on OPT builds - devtools/client/debugger/new/test/mochitest/browser_dbg-tabs.js on DEBUG builds with: | A promise chain failed to handle a rejection: _selectors.getSource(...) is undefined
I ran both tests with --verify but wasn't able to get them to fail. What is the best way to investigate this?
Attached image debugger_close_open_bugs.gif (deleted) —
Some regressions spotted when doing quick manual tests: when closing and reopening the debugger: - source tab is blank - breakpoints are persisted but not visible in the gutter See attached GIF
Comment on attachment 8935336 [details] test The line that makes splinter review fail is the removal of https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/devtools/client/debugger/new/pretty-print-worker.js#3130 I guess we have no choice but to go through mozreview here.
Attachment #8935336 - Attachment is obsolete: true
Attachment #8935336 - Attachment is patch: false
Comment on attachment 8935340 [details] Bug 1423158 - Update Debugger Frontend (12-5). https://reviewboard.mozilla.org/r/206234/#review211816 Looks good to me, some license header missing but be addressed after the release. Found some regressions, commented on bugzilla, feel free to open separate issues to track them (unless already tracked). Should be ready to land once the sourcemap intermittent is fixed. For the debugger tests being intermittent in debug, probably should add skip-if = debug for them and log separate issues to investigate them. ::: devtools/client/debugger/new/test/mochitest/examples/doc-sourcemaps3.html:1 (Diff revision 1) > +<!DOCTYPE html> license header ::: devtools/client/themes/images/debugger/react.svg:1 (Diff revision 1) > +<?xml version="1.0" encoding="utf-8"?> Missing license header
Attachment #8935340 - Flags: review?(jdescottes) → review+
Attached patch fix intermittent (obsolete) (deleted) — — Splinter Review
Comment on attachment 8935340 [details] Bug 1423158 - Update Debugger Frontend (12-5). https://reviewboard.mozilla.org/r/206234/#review211816 > license header do we need a license header on test files? I noticed we don't have it on all of our example test files, should we add it to everything? > Missing license header We are missing license headers for a few of our svgs, i will create an issue for all of them
Attachment #8935351 - Attachment is obsolete: true
Attachment #8934564 - Attachment is obsolete: true
Attachment #8934564 - Flags: review?(jlaster)
Attachment #8934564 - Flags: review?(jdescottes)
Attachment #8935340 - Attachment is obsolete: true
Comment on attachment 8935363 [details] Bug 1423158 - Update Debugger Frontend (12-5) fix intermittent https://reviewboard.mozilla.org/r/206254/#review211862 LGTM, maybe a slightly more helpful commit message could be nice :) ::: commit-message-f532f:1 (Diff revision 1) > +Bug 1423158 - Update Debugger Frontend (12-5) fix intermittent r=jdescottes nit commit message could mention the fix "swallow error for sourcemap failure if toolbox is closed" ::: commit-message-f532f:1 (Diff revision 1) > +Bug 1423158 - Update Debugger Frontend (12-5) fix intermittent r=jdescottes nit: commit message could mention the fix "swallow error for sourcemap failure if toolbox is closed"
Attachment #8935363 - Flags: review?(jdescottes) → review+
Comment on attachment 8934533 [details] Bug 1423158 - Update Debugger Frontend (12-5) https://reviewboard.mozilla.org/r/205440/#review211864
Attachment #8934533 - Flags: review?(jdescottes) → review+
Attachment #8935363 - Attachment is obsolete: true
Need to synchronize the change from Bug 1408949 which updated browser_dbg-toggling-tools.js.
try run looks good!
Pushed by ydelendik@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b7b279271a15 Update Debugger Frontend (12-5) r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/b7b279271a15
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Depends on: 1428586
Attachment #8935333 - Attachment is obsolete: false
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: