Closed
Bug 1438052
Opened 6 years ago
Closed 6 years ago
Uplift fixes for Debugger
Categories
(DevTools :: Debugger, defect)
DevTools
Debugger
Tracking
(firefox59 verified)
VERIFIED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
firefox59 | --- | verified |
People
(Reporter: jlast, Assigned: jlast)
References
Details
Attachments
(1 file, 1 obsolete file)
16.86 KB,
patch
|
jdescottes
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8c1602306bf98a277cff3b146c7a853c093d174
Assignee | ||
Comment 2•6 years ago
|
||
Attachment #8950827 -
Flags: review?(jdescottes)
Assignee | ||
Comment 3•6 years ago
|
||
this patch is still a bit wip, so it doesnt have the beta flag yet here's a comparison though https://github.com/devtools-html/debugger.html/compare/release-9-1...release-9-2
Comment 4•6 years ago
|
||
We already uplifted https://bugzilla.mozilla.org/show_bug.cgi?id=1431127, so we need https://github.com/devtools-html/debugger.html/commit/525e87121c6ccbb7f1caa3ab7a76fab412bc4c49 included in the release. The patch attached here is empty, but I quickly looked at the patch on try. This uplift does not revert the "pause buttons" popup that introduced and that we want to remove for sure from beta. Is this coming in another uplift?
Assignee | ||
Comment 5•6 years ago
|
||
[Feature/Bug causing the regression]: jsx syntax highlighting - https://github.com/devtools-html/debugger.html/pull/5174 disabled breakpoints are not synced - https://github.com/devtools-html/debugger.html/issues/5140 stepping over await calls should not require an ast - https://github.com/devtools-html/debugger.html/issues/5320 stepping should scroll to the correct location - https://github.com/devtools-html/debugger.html/issues/5184 [User impact if declined]: the debugger will be missing some helpful performance and usability fixes [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: no [Is the change risky?]: no [Why is the change risky/not risky?]: it only affects the debugger [String changes made/needed]: none
Attachment #8950827 -
Attachment is obsolete: true
Attachment #8950827 -
Flags: review?(jdescottes)
Attachment #8950981 -
Flags: review?(jdescottes)
Attachment #8950981 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 6•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=69f97841266e0d95c8913afd73713e5830101494
Updated•6 years ago
|
Assignee: nobody → jlaster
Status: NEW → ASSIGNED
Comment 7•6 years ago
|
||
Comment on attachment 8950981 [details] [diff] [review] uplift-2.patch Review of attachment 8950981 [details] [diff] [review]: ----------------------------------------------------------------- Patch looks good to me, thanks Jason! The number of commits being requested for uplift here is relatively small [1] and it disables the breakpoint popup as expected. [1] https://github.com/devtools-html/debugger.html/compare/release-9-1...release-9-2
Attachment #8950981 -
Flags: review?(jdescottes) → review+
status-firefox59:
--- → affected
Comment on attachment 8950981 [details] [diff] [review] uplift-2.patch Improvements to Debugger, we still have ~3 weeks of stabilizing on Beta before RC week, Beta59+
Attachment #8950981 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi Andrei, Tom, could we get a smoke test on Debugger to ensure these changes (which aren't small) do not regression quality? Thanks! Hi Julian, are we expecting more uplifts for debugger in 59? Just wondering about the plans and on what to expect as we get closer to RC week.
Flags: needinfo?(tgrabowski)
Flags: needinfo?(jdescottes)
Flags: needinfo?(andrei.vaida)
Comment 10•6 years ago
|
||
> Hi Julian, are we expecting more uplifts for debugger in 59? Just wondering about the plans and on what to expect as we get closer to RC week.
We might have another, as we just removed some commits from the initial patch here to make it smaller. Forwarding the question to Jason.
Flags: needinfo?(jdescottes) → needinfo?(jlaster)
Comment 11•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/8e0ec8af8f61
Updated•6 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Assignee | ||
Comment 12•6 years ago
|
||
It would be nice to do an additional uplift for some breakpoint saving bugs, I'll try and put something together soon. If we don't get to it, it's alright.
Flags: needinfo?(jlaster)
Comment 13•6 years ago
|
||
I am redirecting Andrei's ni to myself, as I will start the verification process as soon as we have green light from Jason after the additional uplift is done.
Flags: needinfo?(andrei.vaida) → needinfo?(iulia.cristescu)
Updated•6 years ago
|
Flags: needinfo?(tgrabowski)
Comment 15•6 years ago
|
||
Apart from a smoke test across platforms (Windows 10 x64, Ubuntu 16.04 x64, macOs 10.13.2) that focused the main debugger functionality and pottential regressions, I investigated certain scenarios for the breakpoint sync and search. I found some potential problematic cases, but only one of them seems related to these particular fixes. See https://goo.gl/ryh5tV. Please let me know what is your opinion regarding these.
Flags: needinfo?(iulia.cristescu) → needinfo?(jlaster)
Assignee | ||
Comment 16•6 years ago
|
||
Thanks Julia! The overview is fantastic. I think this uplift improved syncing, but there is atleast one case that we had not considered. I've created an issue in github for follow up improvements. https://github.com/devtools-html/debugger.html/issues/5570
Flags: needinfo?(jlaster)
Comment 17•6 years ago
|
||
Confirmed with Jason that the above issue (https://github.com/devtools-html/debugger.html/issues/5570) will be assessed separately, so, based on the previous comments, I will mark this (and also bug 1439753) as verified fixed.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•