Closed Bug 1432842 Opened 6 years ago Closed 6 years ago

Update Debugger Frontend v10


(DevTools :: Debugger, enhancement)

Not set


(firefox60 fixed)

Firefox 60
Tracking Status
firefox60 --- fixed


(Reporter: jlast, Assigned: jlast)




(2 files, 2 obsolete files)

      No description provided.
Assignee: nobody → jlaster
Here is an early try run:

- The patch seems reasonable. about 7K +- lines because we're uprgrading reps.
- We should be careful to not overwrite Gabe's photon changes in debugger.css
here's a new try run w/ the fixes to the mochitests (needed to include another commit w/ a reps bump)
okay, we've got a failure in the optimized builds... not a big deal. will investigate  tomorrow
jlast: FYI the release includes a huge 10MB webpack-stats.html file. Make sure to remove.
Flags: needinfo?(jlaster)
Two issues here:
1/ we regressed the panel.isPaused method, it's now missing!
2/ the browser toolbox test injects in the browser toolbox process:
   - a new-debugger test
   - the new-debugger head.js
   But it doesn't have access to shared-head. And it just happens that waitForPaused in new-debugger head.js was modified to rely on waitUntil, which comes from shared-head. For now I suggest just importing waitUntil, without trying to inject the whole shared-head for this test.

Try run with fixes for both issues:
PR opened for the isPaused method, as I couldn't find it anywhere on the repo.
If try is green with this, I would suggest to make a new release including isPaused and to keep my toolbox test patch separated.
Attached patch release-10.patch (obsolete) — Splinter Review
Flags: needinfo?(jlaster)
Attachment #8945437 - Flags: review?(jdescottes)
Comment on attachment 8945437 [details] [diff] [review]

Review of attachment 8945437 [details] [diff] [review]:

Needs a valid commit, and consistency with the versioning scheme.

::: devtools/client/debugger/new/README.mozilla
@@ +1,4 @@
>  This is the debugger.html project output.
>  See
> +Version 10

10.0 please

@@ +1,5 @@
>  This is the debugger.html project output.
>  See
> +Version 10
> +Commit: -> 404
Attached patch release-10-1.patch (obsolete) — Splinter Review
Attachment #8945437 - Attachment is obsolete: true
Attachment #8945437 - Flags: review?(jdescottes)
Attachment #8945488 - Flags: review?(jdescottes)
Attachment #8945488 - Attachment is obsolete: true
Attachment #8945488 - Flags: review?(jdescottes)
Attachment #8945562 - Flags: review?(jdescottes)
Already reviewed by jlast!
Attachment #8945577 - Flags: review+
Comment on attachment 8945562 [details] [diff] [review]

Review of attachment 8945562 [details] [diff] [review]:

Looks good. Last try push with both patches:
Attachment #8945562 - Flags: review?(jdescottes) → review+
As inbound is currently closed, adding checkin-needed. 
Both patches attached here should land together, the order doesn't matter. Thanks!
Keywords: checkin-needed
Pushed by
Update Debugger Frontend v10. r=jdescottes
duplicate waitUntil() in browser toolbox test;r=jlast
Keywords: checkin-needed
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Blocks: 1430672
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.