Closed
Bug 1370324
Opened 7 years ago
Closed 7 years ago
Update Debugger frontend (6/5/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, 4 obsolete files)
978.60 KB,
patch
|
jdescottes
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8874544 -
Flags: review?(jdescottes)
Assignee | ||
Comment 2•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f6f1e5d3f7399077562d663dd62706a267ef836
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8874544 -
Attachment is obsolete: true
Attachment #8874544 -
Flags: review?(jdescottes)
Attachment #8874653 -
Flags: review?(jdescottes)
Assignee | ||
Comment 4•7 years ago
|
||
Attachment #8874653 -
Attachment is obsolete: true
Attachment #8874653 -
Flags: review?(jdescottes)
Attachment #8874656 -
Flags: review?(jdescottes)
Comment 5•7 years ago
|
||
Comment on attachment 8874656 [details] [diff] [review] 6-5-2.patch Review of attachment 8874656 [details] [diff] [review]: ----------------------------------------------------------------- - the commit message doesn't mention any sha1/tag - patch needs rebasing - few nits Some additional questions: - the previous try run had failures for browser_dbg-breaking.js, I suppose the last patches attached are addressing this? - the patch is unusually large here (3.6MB), is it normal? Thanks! ::: devtools/client/debugger/new/test/mochitest/.eslintrc @@ +25,5 @@ > "requestLongerTimeout": false, > "SimpleTest": false, > "SpecialPowers": false, > "TestUtils": false, > + "thisTestLeaksUncaughtRejectionsAndShouldBeFixed": false, Your patch doesn't apply cleanly because of this file, please rebase. ::: devtools/client/debugger/new/test/mochitest/browser.ini @@ +50,5 @@ > [browser_dbg-expressions.js] > [browser_dbg-scopes.js] > [browser_dbg-chrome-create.js] > [browser_dbg-chrome-debugging.js] > +skip-if = true Is there an issue/bug tracking this? ::: devtools/client/debugger/new/test/mochitest/head.js @@ +31,5 @@ > * @module mochitest/helpers > * @parent mochitest > */ > > +// shared-head.js handles ? Should be restored I guess ::: devtools/client/themes/toolbox.css @@ +171,5 @@ > background-color: var(--theme-toolbar-hover-active); > } > > +.devtools-tab:not(.selected).highlighted { > + background-color: var(--theme-toolbar-background-alt) missing semi-colon ::: devtools/client/themes/variables.css @@ +149,5 @@ > --theme-sidebar-background: #fcfcfc; > --theme-contrast-background: #e6b064; > > --theme-tab-toolbar-background: rgb(240, 240, 240) linear-gradient(rgba(255, 255, 255, 0.8), transparent); > + --theme-toolbar-background-hover: rgba(221, 225, 228, 0.66); FYI, firebug theme inherits light theme, so if the value is exactly the same you don't have to define it.
Attachment #8874656 -
Flags: review?(jdescottes)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8874656 -
Attachment is obsolete: true
Attachment #8874827 -
Flags: review?(jdescottes)
Comment 7•7 years ago
|
||
Comment on attachment 8874827 [details] [diff] [review] 6-5-3.patch Review of attachment 8874827 [details] [diff] [review]: ----------------------------------------------------------------- Bundle looks good, as discussed we spotted two bugs: - breakpoints are sliding when reloading the content page - call stack is not showing all the frames when disabling framework grouping Waiting for a bundle addressing the first issue at least.
Attachment #8874827 -
Flags: review?(jdescottes)
Assignee | ||
Comment 8•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ef668f361c788de2a75d51dea21066396a240ab
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #8874827 -
Attachment is obsolete: true
Attachment #8874949 -
Flags: review?(jdescottes)
Assignee | ||
Comment 10•7 years ago
|
||
Thanks julian, I've fixed the first issue. We can punt on the call stack. Few people use it and I don't think it will be noticed before the next release goes out.
Assignee | ||
Comment 11•7 years ago
|
||
I forgot to mention, but the newest patch is much smaller. Ryan found the regression and fixed it here: https://github.com/devtools-html/debugger.html/pull/3104
Comment 12•7 years ago
|
||
Comment on attachment 8874949 [details] [diff] [review] 6-5-4.patch Review of attachment 8874949 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me if try is green, thanks! ::: devtools/client/debugger/new/debugger.css @@ +1646,5 @@ > cursor: pointer; > } > > .selected-token { > + background-color: var(--theme-highlight-yellow); --theme-search-overlays-semitransparent is no longer used anywhere and should be removed from variables.css @@ +1748,5 @@ > +.theme-light { > + --theme-conditional-breakpoint-color: #ccd1d5; > +} > + > +.theme-firebug { not only could this be rewritten as .theme-light, .theme firebug {}, but as mentioned earlier theme firebug inherits light theme.
Attachment #8874949 -
Flags: review?(jdescottes) → review+
Comment 13•7 years ago
|
||
(In reply to Jason Laster [:jlast] from comment #11) > I forgot to mention, but the newest patch is much smaller. Ryan found the > regression and fixed it here: > https://github.com/devtools-html/debugger.html/pull/3104 Great job!
Comment 14•7 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f58a80807c73 Update Debugger frontend (v0.5) (6/5/2017). r=jdescottes
Comment 15•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f58a80807c73
Assignee: nobody → jlaster
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
•