Closed
Bug 1338567
Opened 7 years ago
Closed 7 years ago
Update Frontend (2/10/2017)
Categories
(DevTools :: Debugger, defect, P3)
DevTools
Debugger
Tracking
(firefox54 fixed)
RESOLVED
FIXED
Firefox 54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: jlast, Assigned: jlast)
References
Details
Attachments
(2 files, 16 obsolete files)
4.68 KB,
patch
|
jlast
:
review+
|
Details | Diff | Splinter Review |
4.42 MB,
patch
|
jdescottes
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jlaster
Priority: -- → P3
Assignee | ||
Comment 1•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b7d6ec34e801aab6f787fa49fdcdbe4778ecd12
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8836100 -
Flags: review?(jdescottes)
Comment 3•7 years ago
|
||
Comment on attachment 8836100 [details] [diff] [review] bundle2-10.patch Review of attachment 8836100 [details] [diff] [review]: ----------------------------------------------------------------- The patch contains several new map files: - debugger.css.map - debugger.js.map - integration-tests.js.map - pretty-print-worker.js.map - source-map-worker.js.map Do we really want them in mc now? We have a few things to cleanup in debugger.properties. Forwarding a review request to flod: can you have a look at the changes to debugger.properties? In particular there are two strings for which the key changed but the string didn't. What's the recommendation, can we update the keys here? ::: devtools/client/debugger/new/test/mochitest/browser.ini @@ +43,3 @@ > [browser_dbg-breaking.js] > [browser_dbg-breaking-from-console.js] > +skip-if = true Could we have a comment here explaining why the test is skipped, with a link to a bug/issue if any is available? ::: devtools/client/debugger/new/test/mochitest/head.js @@ +530,5 @@ > * @param {String} fnc > * @return {Promise} > * @static > */ > + nit: remove extra line ::: devtools/client/locales/en-US/debugger.properties @@ +17,5 @@ > +# LOCALIZATION NOTE (copySourceUrl): This is the text that appears in the > +# context menu to copy the source URL of file open. > +copySourceUrl=Copy Source Url > + > +# LOCALIZATION NOTE (copySourceUrl): This is the text that appears in the Localization note is wrong. @@ +27,5 @@ > expandPanes=Expand panes > > # LOCALIZATION NOTE (pauseButtonTooltip): The tooltip that is displayed for the pause > # button when the debugger is in a running state. > +pauseButtonTooltip=Click to pause %S update the localization key or revert the label @@ +35,5 @@ > pausePendingButtonTooltip=Waiting for next execution > > # LOCALIZATION NOTE (resumeButtonTooltip): The label that is displayed on the pause > # button when the debugger is in a paused state. > +resumeButtonTooltip=Click to resume %S update the localization key or revert the label @@ +267,2 @@ > # for navigating to a source mapped location > +editor.jumpToMappedLocation=Jump to %S location Not sure we should change the localization key if the string did not change. (same for sourceTabs.closeTabsToRight -> sourceTabs.closeTabsToEnd) @@ +288,5 @@ > # LOCALIZATION NOTE (sourceTabs.closeOtherTabs): Editor source tab context menu item > # for closing the other tabs. > sourceTabs.closeOtherTabs=Close others > > +# LOCALIZATION NOTE (sourceTabs.closeOtherTabs): Editor source tab context menu item Localization note is wrong @@ +296,5 @@ > +# LOCALIZATION NOTE (sourceTabs.closeTabsToEnd): Editor source tab context menu item > +# for closing the tabs to the end (the right for LTR languages) of the selected tab. > +sourceTabs.closeTabsToEnd=Close tabs to the right > + > +# LOCALIZATION NOTE (sourceTabs.closeTabsToEnd): Editor source tab context menu item Localization note is wrong @@ +304,5 @@ > # LOCALIZATION NOTE (sourceTabs.closeAllTabs): Editor source tab context menu item > # for closing all tabs. > sourceTabs.closeAllTabs=Close all tabs > > +# LOCALIZATION NOTE (sourceTabs.closeAllTabs): Editor source tab context menu item Localization note is wrong @@ +312,5 @@ > +# LOCALIZATION NOTE (sourceTabs.revealInTree): Editor source tab context menu item > +# for revealing source in tree. > +sourceTabs.revealInTree=Reveal in Tree > + > +# LOCALIZATION NOTE (sourceTabs.revealInTree): Editor source tab context menu item Localization note is wrong @@ +320,5 @@ > +# LOCALIZATION NOTE (sourceTabs.copyLink): Editor source tab context menu item > +# for copying a link address. > +sourceTabs.copyLink=Copy Link Address > + > +# LOCALIZATION NOTE (sourceTabs.copyLink): Editor source tab context menu item Localization note is wrong @@ +328,5 @@ > +# LOCALIZATION NOTE (sourceTabs.prettyPrint): Editor source tab context menu item > +# for pretty printing the source. > +sourceTabs.prettyPrint=Pretty Print Source > + > +# LOCALIZATION NOTE (sourceTabs.prettyPrint): Editor source tab context menu item Localization note is wrong
Attachment #8836100 -
Flags: review?(jdescottes)
Attachment #8836100 -
Flags: review?(francesco.lodolo)
Attachment #8836100 -
Flags: review+
Comment 4•7 years ago
|
||
Comment on attachment 8836100 [details] [diff] [review] bundle2-10.patch Review of attachment 8836100 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/locales/en-US/debugger.properties @@ +27,5 @@ > expandPanes=Expand panes > > # LOCALIZATION NOTE (pauseButtonTooltip): The tooltip that is displayed for the pause > # button when the debugger is in a running state. > +pauseButtonTooltip=Click to pause %S I start to feel like quite lost. I've just commented a PR on GitHub which, at this point, seems related to this bug. This is particularly confusing (same for resumeButtonTooltip): 1. The original string was "Click to pause %S" (it still is in mozilla-beta) 2. Then it changed to "Pause %S" without a new ID (currently in mozilla-aurora) 3. Filed a bug to fix it, the ID was changed until we realized that we needed to keep the old one around for the old debugger. So this was reverted. If you really want to use "Click to pause %S" for the new debugger, at this point use a new string ID, but keep the old one around (for the old debugger). @@ +267,2 @@ > # for navigating to a source mapped location > +editor.jumpToMappedLocation=Jump to %S location As commented on GH, this one needs to remain editor.jumpToMappedLocation1 (and localization note fixed) editor.jumpToMappedLocation is already available in mozilla-aurora, and has a %s that we want to get rid of. @@ +294,5 @@ > +sourceTabs.closeOtherTabs.key=O > + > +# LOCALIZATION NOTE (sourceTabs.closeTabsToEnd): Editor source tab context menu item > +# for closing the tabs to the end (the right for LTR languages) of the selected tab. > +sourceTabs.closeTabsToEnd=Close tabs to the right Not really necessary, but I guess it's OK to have a less LTR string ID.
Attachment #8836100 -
Flags: review?(francesco.lodolo) → review-
Comment 5•7 years ago
|
||
Thanks for the feedback on GH and here flod. Looks like we have some synchronization to do between the mozilla-central and github before we release this new bundle. Mainly I think I missed to port the changes from Bug 1334154 to GitHub. Let's move the conversation to https://github.com/devtools-html/debugger.html/pull/1987 and we will reupdate the bundle once everything is ok there.
Assignee | ||
Comment 6•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2667774617977e21ff6cd1fd65bba5c921e785e2
Assignee | ||
Comment 7•7 years ago
|
||
Attachment #8836100 -
Attachment is obsolete: true
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8836344 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/mozilla-inbound/rev/c8f8a1b5dfeb Update debugger frontend (2/10/2017) r=jdescottes.
Keywords: checkin-needed
Comment 10•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/415e2bd11f41825e78c374851091364c354bacf1 essentially for being a giant unmergeable blob. The only thing in https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=43270c88b2513a6758a0ce6669559f4a5f3c9207 that I should have been having to merge with it was https://hg.mozilla.org/mozilla-central/rev/befb3e282a53f86294308110ad51049c2f80647c which ought to have been handled fine in hg's internal merge, but instead I got dumped into a 3-way claiming (once my merge app finally managed to digest that push) that I had to merge all sorts of incomprehensible things that shouldn't have been conflicts.
Comment 11•7 years ago
|
||
Since this got backed we might want to get a new bundle including https://github.com/devtools-html/debugger.html/pull/2003 (fixed a localization note I missed the other day).
Assignee | ||
Comment 12•7 years ago
|
||
Attachment #8836347 -
Attachment is obsolete: true
Attachment #8840643 -
Flags: review?(jdescottes)
Assignee | ||
Comment 13•7 years ago
|
||
Attachment #8840643 -
Attachment is obsolete: true
Attachment #8840643 -
Flags: review?(jdescottes)
Attachment #8840649 -
Flags: review?(jdescottes)
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8840651 -
Flags: review?(gerv)
Comment 15•7 years ago
|
||
Comment 16•7 years ago
|
||
Comment 17•7 years ago
|
||
Comment 18•7 years ago
|
||
Comment on attachment 8840649 [details] [diff] [review] bundle2-10-5.patch Review of attachment 8840649 [details] [diff] [review]: ----------------------------------------------------------------- I mostly reviewed the properties file, but the code change looks good! While testing I found some UI regressions (see the attached screenshots): - text is too dark in dark theme - icons in the tabbar area are too small - background color of the selected source in the source tree is too small At the very least I think the dark theme issue should be fixed. Clearing the review flag while waiting for an updated bundle.
Attachment #8840649 -
Flags: review?(jdescottes)
Comment 19•7 years ago
|
||
Comment on attachment 8840649 [details] [diff] [review] bundle2-10-5.patch Review of attachment 8840649 [details] [diff] [review]: ----------------------------------------------------------------- There's a lot of confusion in debugger.properties. If it's a shortcut, it's OK to call the string .key, but the localization comment should make it clear it's a shortcut. If it's an accesskey, it should be called correspondinglabel.accesskey ::: devtools/client/locales/en-US/debugger.properties @@ +19,5 @@ > +copySourceUrl=Copy Source Url > + > +# LOCALIZATION NOTE (copySourceUrl.key): Access key to copy the source URL of a file from > +# the context menu. > +copySourceUrl.key=X Is this an accesskey or a shortcut? It sounds like the latter. If it's an accesskey, it should be copySourceUrl.accesskey @@ +140,2 @@ > # the number of matches for autocomplete > +sourceSearch.resultsSummary1=%d results Have we figured out the story about plural? See also another bug that I filed recently.
Attachment #8840649 -
Flags: feedback-
Comment 20•7 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #19) > Comment on attachment 8840649 [details] [diff] [review] > bundle2-10-5.patch > > Review of attachment 8840649 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: devtools/client/locales/en-US/debugger.properties > @@ +19,5 @@ > > +copySourceUrl=Copy Source Url > > + > > +# LOCALIZATION NOTE (copySourceUrl.key): Access key to copy the source URL of a file from > > +# the context menu. > > +copySourceUrl.key=X > > Is this an accesskey or a shortcut? It sounds like the latter. > > If it's an accesskey, it should be copySourceUrl.accesskey It is a context-menu accesskey. Good point about the naming, I wasn't aware of this rule though :/ Do we actually need localization notes for access keys then? If using "accesskey" has meaning, I think the localization note becomes redundant.
Assignee | ||
Comment 21•7 years ago
|
||
:flod i'll clean up the accessKey labeling. We have not figured out the pluralization support yet. :jdescottes i'll file an issue for the dark theme regressions
Assignee | ||
Comment 22•7 years ago
|
||
Attachment #8840649 -
Attachment is obsolete: true
Attachment #8841108 -
Flags: review?(jdescottes)
Assignee | ||
Comment 23•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7660ed74e24726afda421b376ef69ae88351a01
Assignee | ||
Comment 24•7 years ago
|
||
Julian, we've localized the access keys because we imagine they're shorthand for some actions that like "copy filename" where the access key "c" could be translated to another letter. Happy to revert and remove them too. What do you mean by redundant? also, created this issue to discuss the tab icons: https://github.com/devtools-html/debugger.html/issues/2141
Assignee | ||
Comment 25•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7660ed74e24726afda421b376ef69ae88351a01
Comment 26•7 years ago
|
||
Comment on attachment 8841108 [details] [diff] [review] bundle2-10-6.patch Review of attachment 8841108 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/locales/en-US/debugger.properties @@ +19,5 @@ > +copySourceUrl=Copy Source Url > + > +# LOCALIZATION NOTE (copySourceUrl.accessKey): Access key to copy the source URL of a file from > +# the context menu. > +copySourceUrl.accessKey=X If this is an accesskey, you should pick a character available in the label, unless that context menu doesn't have any free left. @@ +290,5 @@ > sourceTabs.closeOtherTabs=Close others > > +# LOCALIZATION NOTE (sourceTabs.closeOtherTabs.accesKey): Access key to close other source tabs > +# from the editor context menu. > +sourceTabs.closeOtherTabs.accesKey=O Nit: o, not O, since the label is "Close tab" (no practical effect change, just clearer) @@ +298,5 @@ > +sourceTabs.closeTabsToEnd=Close tabs to the right > + > +# LOCALIZATION NOTE (sourceTabs.closeTabsToEnd.accessKey): Access key to close source tabs > +# after the selected tab from the editor context menu. > +sourceTabs.closeTabsToEnd.accessKey=R R -> r In general, you can avoid a localization comment for this kind of strings, simply put this string right after the label. @@ +306,5 @@ > sourceTabs.closeAllTabs=Close all tabs > > +# LOCALIZATION NOTE (sourceTabs.closeAllTabs.accessKey): Access key to close all tabs from the > +# editor context menu. > +sourceTabs.closeAllTabs.accessKey=A A -> a @@ +312,5 @@ > +# LOCALIZATION NOTE (sourceTabs.revealInTree): Editor source tab context menu item > +# for revealing source in tree. > +sourceTabs.revealInTree=Reveal in Tree > + > +# LOCALIZATION NOTE (sourceTabs.revealInTree.key): Access key to reveal a source in the access key -> shortcut @@ +320,5 @@ > +# LOCALIZATION NOTE (sourceTabs.copyLink): Editor source tab context menu item > +# for copying a link address. > +sourceTabs.copyLink=Copy Link Address > + > +# LOCALIZATION NOTE (sourceTabs.copyLink.key): Access key to copy a link addresss from the access key -> shortcut @@ +328,5 @@ > +# LOCALIZATION NOTE (sourceTabs.prettyPrint): Editor source tab context menu item > +# for pretty printing the source. > +sourceTabs.prettyPrint=Pretty Print Source > + > +# LOCALIZATION NOTE (sourceTabs.prettyPrint.key): Access key to pretty print a source from access key -> shortcut
Assignee | ||
Comment 27•7 years ago
|
||
Attachment #8840672 -
Attachment is obsolete: true
Attachment #8840673 -
Attachment is obsolete: true
Attachment #8840675 -
Attachment is obsolete: true
Attachment #8841108 -
Attachment is obsolete: true
Attachment #8841108 -
Flags: review?(jdescottes)
Attachment #8841274 -
Flags: review?(jdescottes)
Assignee | ||
Comment 28•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebcc99b1e37ba56e5996bd2bacb6ddf7051c5c8b
Comment 29•7 years ago
|
||
(In reply to Jason Laster [:jlast] from comment #24) > Julian, we've localized the access keys because we imagine they're shorthand > for some actions that like "copy filename" where the access key "c" could be > translated to another letter. Happy to revert and remove them too. > > What do you mean by redundant? > > also, created this issue to discuss the tab icons: > https://github.com/devtools-html/debugger.html/issues/2141 I am talking about "localization notes" not the localization itself. My point is that adding a comment such as "this is the access for XYZ ..." is not really helpful if localizers know that my.string.accesskey is an access key for my.string. flod answered this question in his review, looks like the loc. notes are not mandatory.
Comment 30•7 years ago
|
||
Comment on attachment 8841274 [details] [diff] [review] bundle2-10-7.patch Review of attachment 8841274 [details] [diff] [review]: ----------------------------------------------------------------- R+ as this fixes most of the dark theme issues. Note that 2 items are still too dark in dark theme: - icons in the source tree (almost the same color as the background color) - close icon in a source tab (black) Let's forward the review request to flod to finalize the debugger.properties review. I feel like I will keep missing things here. ::: devtools/client/locales/en-US/debugger.properties @@ +19,5 @@ > +copySourceUrl=Copy Source Url > + > +# LOCALIZATION NOTE (copySourceUrl.accessKey): Access key to copy the source URL of a file from > +# the context menu. > +copySourceUrl.accessKey=X in all the properties files in devtools, I found "accesskey" and "not "accessKey". Need to check with flod if the case can be an issue. @@ +290,5 @@ > sourceTabs.closeOtherTabs=Close others > > +# LOCALIZATION NOTE (sourceTabs.closeOtherTabs.accesKey): Access key to close other source tabs > +# from the editor context menu. > +sourceTabs.closeOtherTabs.accesKey=O accesKey -> accesskey @@ +314,5 @@ > +sourceTabs.revealInTree=Reveal in Tree > + > +# LOCALIZATION NOTE (sourceTabs.revealInTree.key): Access key to reveal a source in the > +# tree from the context menu. > +sourceTabs.revealInTree.key=s I think this is an access key and not a shortcut? so key -> accesskey @@ +322,5 @@ > +sourceTabs.copyLink=Copy Link Address > + > +# LOCALIZATION NOTE (sourceTabs.copyLink.key): Access key to copy a link addresss from the > +# editor context menu. > +sourceTabs.copyLink.key=X same, this is an access key and not a shortcut? @@ +330,5 @@ > +sourceTabs.prettyPrint=Pretty Print Source > + > +# LOCALIZATION NOTE (sourceTabs.prettyPrint.key): Access key to pretty print a source from > +# the editor context menu. > +sourceTabs.prettyPrint.key=Z same, this is an access key and not a shortcut?
Attachment #8841274 -
Flags: review?(jdescottes)
Attachment #8841274 -
Flags: review?(francesco.lodolo)
Attachment #8841274 -
Flags: review+
Comment 31•7 years ago
|
||
(just found that styleinspector.properties also uses accessKey instead of accesskey, so maybe this one is not an issue)
Comment 32•7 years ago
|
||
Comment on attachment 8841274 [details] [diff] [review] bundle2-10-7.patch Review of attachment 8841274 [details] [diff] [review]: ----------------------------------------------------------------- The problem remains across the file: * if it's an accesskey, the ID should have .accesskey, the character should be available in the connected label (in the same case for clarity). * if it's a shortcut, the ID can use .key, no constraints on the character you pick besides conflicts. ::: devtools/client/locales/en-US/debugger.properties @@ +19,5 @@ > +copySourceUrl=Copy Source Url > + > +# LOCALIZATION NOTE (copySourceUrl.accessKey): Access key to copy the source URL of a file from > +# the context menu. > +copySourceUrl.accessKey=X Both should work https://github.com/translate/translate/blob/c713f0c6c93bbffdcd960cd82ef983fd10b4c541/translate/storage/properties.py#L134 But for new strings, let's stick to lowercase .accesskey copySourceUrl.accesskey= (some letter available in the label) @@ +290,5 @@ > sourceTabs.closeOtherTabs=Close others > > +# LOCALIZATION NOTE (sourceTabs.closeOtherTabs.accesKey): Access key to close other source tabs > +# from the editor context menu. > +sourceTabs.closeOtherTabs.accesKey=O sourceTabs.closeOtherTabs.accesskey=o @@ +298,5 @@ > +sourceTabs.closeTabsToEnd=Close tabs to the right > + > +# LOCALIZATION NOTE (sourceTabs.closeTabsToEnd.accessKey): Access key to close source tabs > +# after the selected tab from the editor context menu. > +sourceTabs.closeTabsToEnd.accessKey=R sourceTabs.closeTabsToEnd.accesskey=r @@ +314,5 @@ > +sourceTabs.revealInTree=Reveal in Tree > + > +# LOCALIZATION NOTE (sourceTabs.revealInTree.key): Access key to reveal a source in the > +# tree from the context menu. > +sourceTabs.revealInTree.key=s sourceTabs.revealInTree.accesskey= (some letter available in the label) @@ +322,5 @@ > +sourceTabs.copyLink=Copy Link Address > + > +# LOCALIZATION NOTE (sourceTabs.copyLink.key): Access key to copy a link addresss from the > +# editor context menu. > +sourceTabs.copyLink.key=X sourceTabs.copyLink.accesskey= (some letter available in the label) @@ +330,5 @@ > +sourceTabs.prettyPrint=Pretty Print Source > + > +# LOCALIZATION NOTE (sourceTabs.prettyPrint.key): Access key to pretty print a source from > +# the editor context menu. > +sourceTabs.prettyPrint.key=Z sourceTabs.prettyPrint.accesskey= (some letter available in the label) @@ +504,5 @@ > # LOCALIZATION NOTE (functionSearchSeparatorLabel): The text that is displayed > # in the functions search panel as a separator between function's inferred name > # and its real name (if available). > functionSearchSeparatorLabel=← > +functionSearch.search.placeholder=Search Functions... Use … (single unicode character), not ...
Attachment #8841274 -
Flags: review?(francesco.lodolo) → review-
Comment 33•7 years ago
|
||
Comment on attachment 8840651 [details] [diff] [review] bug1338567.licences.patch Review of attachment 8840651 [details] [diff] [review]: ----------------------------------------------------------------- r+ with these nits addressed. Gerv ::: toolkit/content/license.html @@ +2576,5 @@ > + </p> > + > +<pre> > +MIT License > + Please remove the above two lines (blank and "MIT"). @@ +2605,5 @@ > + <h1><a id="babylon"></a>Babylon License</h1> > + > + <p>This license applies to files in the directory > + <span class="path">devtools/client/debugger/new/debugger.js/</span>. > + </p> This is the same path as the previous license; do you mean "some files" in each case? @@ +2609,5 @@ > + </p> > + > +<pre> > +Copyright (C) 2012-2014 by various contributors (see AUTHORS) > + Is the AUTHORS file present in that directory? If not, please add it.
Attachment #8840651 -
Flags: review?(gerv) → review+
Assignee | ||
Comment 34•7 years ago
|
||
Attachment #8841274 -
Attachment is obsolete: true
Assignee | ||
Comment 35•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3918fd199ef545c4e3099c950a6da13df4b6f355
Assignee | ||
Comment 36•7 years ago
|
||
Attachment #8840651 -
Attachment is obsolete: true
Assignee | ||
Comment 37•7 years ago
|
||
Attachment #8841721 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•7 years ago
|
Attachment #8841972 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Attachment #8841723 -
Flags: review+
Comment 38•7 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9dcba1a87af Update debugger frontend (2/10/2017) r=jdescottes.
Keywords: checkin-needed
Comment 39•7 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c594a1353f5d add licenses for new debugger dependencies;r=gerv
I had to back these out because they seem to have caused various debugger failures like https://treeherder.mozilla.org/logviewer.html#?job_id=80885044&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/df4752809a83
Flags: needinfo?(jlaster)
Comment 41•7 years ago
|
||
Looks like we have a new intermittent with browser_dbg-sourcemaps-bogus.js. Checking the logs, the test actually seems to complete successfully but times out way over the usual limit: over 100s, while the timeout is set at 45s. We might have to disable this test on this platform (OSX debug). First of all let's see if skipping this test helps with the intermittent failures or if it just moves the failure to another test: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3480a4e2dcf5e4abc1f51d2576b7986d567000b6
Flags: needinfo?(jlaster)
Comment 42•7 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c08af8aad4ce add licenses for new debugger dependencies;r=gerv https://hg.mozilla.org/integration/mozilla-inbound/rev/92469a4ff6f4 Update debugger frontend (2/10/2017) r=jdescottes.
Comment 43•7 years ago
|
||
Disabled two bogus mochitests for the new debugger, try looks green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=97143756265eccb7fa6aa5ed80c2acb4ea05ef92
Comment 44•7 years ago
|
||
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/56f83340ce93 Backed out changeset 92469a4ff6f4 https://hg.mozilla.org/integration/mozilla-inbound/rev/8dd5417e8cb5 Backed out changeset c08af8aad4ce on developers request
Comment 45•7 years ago
|
||
I don't really know what we can do for this bundle except skipping all the new debugger tests in debug mode: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0efdf6e5a40d1b33b2a38ab7c8ce92ddab0a0e80 Jason: I made a PR on the debugger to synchronize this change [1]. You mentioned you wanted to do a new debugger release today. If it's still the case feel free to merge the PR and go ahead with the new bundle. [1] https://github.com/devtools-html/debugger.html/pull/2254/files
Flags: needinfo?(jlaster)
Comment 46•7 years ago
|
||
Here's the bundle I pushed to try. As I mentioned it skips all the new debugger tests on debug platforms. If we go forward with this (and I guess we will) this issue really needs to be addressed before we consider enabling the debugger past Nightly.
Attachment #8841972 -
Attachment is obsolete: true
Attachment #8842835 -
Flags: review+
Assignee | ||
Comment 47•7 years ago
|
||
Attachment #8842835 -
Attachment is obsolete: true
Flags: needinfo?(jlaster)
Attachment #8843020 -
Flags: review?(jdescottes)
Assignee | ||
Comment 48•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a922e8172be74944e6f2ff46746784d0951baace
Assignee | ||
Comment 49•7 years ago
|
||
Attachment #8843020 -
Attachment is obsolete: true
Attachment #8843020 -
Flags: review?(jdescottes)
Attachment #8843050 -
Flags: review?(jdescottes)
Assignee | ||
Comment 50•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb058c63b9c43a1e9307b68691078986c1c15c4b ^ removed integration-tests.js as it wasn't being used
Comment 51•7 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8e76e3bf6ce3 add licenses for new debugger dependencies;r=gerv https://hg.mozilla.org/integration/mozilla-inbound/rev/0598ed8d643b Update debugger frontend (2/10/2017) r=jdescottes.
Comment 52•7 years ago
|
||
Let's go with the previous bundle, skipping debug tests.
Attachment #8843050 -
Attachment is obsolete: true
Attachment #8843050 -
Flags: review?(jdescottes)
Attachment #8843105 -
Flags: review+
Comment 53•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8e76e3bf6ce3 https://hg.mozilla.org/mozilla-central/rev/0598ed8d643b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•