Closed Bug 1472023 Opened Last year Closed Last year

Update Debugger Frontend v66

Categories

(DevTools :: Debugger, enhancement, P3)

enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: jlast, Assigned: jlast)

References

Details

Attachments

(2 files, 4 obsolete files)

Attached patch 66-1.patch (obsolete) — Splinter Review
Attachment #8988618 - Flags: review?(dwalsh)
Comment on attachment 8988618 [details] [diff] [review]
66-1.patch

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

Looks like devtools/client/shared/components/reps/reps.js does not have all the changes it should have (i'm thinking of the Services addition in OI, rollback of replacement of explicitOriginalTarget, …). Not sure how the reps bundle was generated
Attached patch 66-2.patch (obsolete) — Splinter Review
Attachment #8988618 - Attachment is obsolete: true
Attachment #8988618 - Flags: review?(dwalsh)
Attachment #8988704 - Flags: review?(dwalsh)
Comment on attachment 8988704 [details] [diff] [review]
66-2.patch

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

- I'm seeing the following error in console:

```shell
JavaScript error: resource://devtools/shared/base-loader.js -> resource://devtools/client/debugger/new/src/components/Editor/Preview/Popup.js, line 67: TypeError: relatedTarget.classList is undefined
````

- The README needs an update

- I'm seeing this other breakpoint highlight issue which I don't think is specific to the release but worth noting.  Will add a screenshot.
Attachment #8988704 - Flags: review?(dwalsh) → review-
Attached patch release-66.patch (obsolete) — Splinter Review
Attachment #8988704 - Attachment is obsolete: true
Attachment #8988736 - Attachment is obsolete: true
Attachment #8988776 - Flags: review?(jdescottes)
Comment on attachment 8988776 [details] [diff] [review]
release-66.patch

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

LGTM, did you submit to try? 

Will rebase and land with a green try.

::: devtools/client/debugger/new/test/mochitest/browser.ini
@@ +157,4 @@
>  skip-if = ccov || (verify && debug && (os == 'linux')) # Bug 1441545
>  [browser_dbg-sourcemapped-stepping.js]
>  [browser_dbg-sourcemapped-preview.js]
> +skip-if = os == "win" # Bug 1448523, Bug 1448450

You need to update your mozilla-central to a more recent revision, this diff should not show up and will prevent us from landing.
Attachment #8988776 - Flags: review?(jdescottes) → review+
David: the try push seems to have a lot of failures. I don't know if my rebased was faulty or something, but can you try to run:

devtools/client/debugger/new/test/mochitest/browser_dbg-wasm-sourcemaps.js
devtools/client/webconsole/test/mochitest/browser_webconsole_object_inspector_scroll.js

and see if they pass for you?
Flags: needinfo?(dwalsh)
Comment on attachment 8988776 [details] [diff] [review]
release-66.patch

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

::: devtools/client/shared/components/reps/reps.js
@@ +4440,5 @@
>  
>          // Only set default focus to the first tree node if the focus came
>          // from outside the tree (e.g. by tabbing to the tree from other
>          // external elements).
> +        if (relatedTarget !== this.treeRef && !this.treeRef.contains(relatedTarget)) {

Actually I missed this, it seems to be a missing sync from https://bugzilla.mozilla.org/show_bug.cgi?id=1469959

It was synced back by Nicolas on debugger.html with https://github.com/devtools-html/debugger.html/commit/ca4daa81f761087f9fa6ca95a677808a4d013118 . It needs to be included in the release as well.
Attached patch 66-3.patchSplinter Review
Attachment #8988776 - Attachment is obsolete: true
Attachment #8989195 - Flags: review+
Attached patch 66-2-1.patchSplinter Review
Attachment #8989232 - Flags: review+
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1f20fc7d71b
Update Debugger Frontend v66 (enable wasm test). r=dwalsh
https://hg.mozilla.org/mozilla-central/rev/5e463b28e200
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Flags: needinfo?(jlaster)
Flags: needinfo?(dwalsh)
You need to log in before you can comment on or make changes to this bug.