Closed Bug 1338567 Opened 7 years ago Closed 7 years ago

Update Frontend (2/10/2017)

Categories

(DevTools :: Debugger, defect, P3)

defect

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: nobody → jlaster
Priority: -- → P3
Attached patch bundle2-10.patch (obsolete) — Splinter Review
Attachment #8836100 - Flags: review?(jdescottes)
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 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-
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.
Attached patch bundle2-10-2.patch (obsolete) — Splinter Review
Attachment #8836100 - Attachment is obsolete: true
Attached patch bundle2-10-3.patch (obsolete) — Splinter Review
Attachment #8836344 - Attachment is obsolete: true
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
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.
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).
Attached patch bundle2-10-4.patch (obsolete) — Splinter Review
Attachment #8836347 - Attachment is obsolete: true
Attachment #8840643 - Flags: review?(jdescottes)
Attached patch bundle2-10-5.patch (obsolete) — Splinter Review
Attachment #8840643 - Attachment is obsolete: true
Attachment #8840643 - Flags: review?(jdescottes)
Attachment #8840649 - Flags: review?(jdescottes)
Attached patch bug1338567.licences.patch (obsolete) — Splinter Review
Attachment #8840651 - Flags: review?(gerv)
Attached image debugger_dark_theme.png (obsolete) —
Attached image debugger_icons_size.png (obsolete) —
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 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-
(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.
: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
Blocks: 1326837
Attached patch bundle2-10-6.patch (obsolete) — Splinter Review
Attachment #8840649 - Attachment is obsolete: true
Attachment #8841108 - Flags: review?(jdescottes)
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
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
Attached patch bundle2-10-7.patch (obsolete) — Splinter Review
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)
(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 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+
(just found that styleinspector.properties also uses accessKey instead of accesskey, so maybe this one is not an issue)
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 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+
Attached patch bundle2-10-8.patch (obsolete) — Splinter Review
Attachment #8841274 - Attachment is obsolete: true
Attached patch licences-2.patchSplinter Review
Attachment #8840651 - Attachment is obsolete: true
Attached patch bundle2-10-9.patch (obsolete) — Splinter Review
Attachment #8841721 - Attachment is obsolete: true
Keywords: checkin-needed
Attachment #8841972 - Flags: review+
Attachment #8841723 - Flags: review+
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
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c594a1353f5d
add licenses for new debugger dependencies;r=gerv
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)
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.
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
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)
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+
Attached patch bundle2-10-10.patch (obsolete) — Splinter Review
Attachment #8842835 - Attachment is obsolete: true
Flags: needinfo?(jlaster)
Attachment #8843020 - Flags: review?(jdescottes)
Attached patch bundle2-10-11.patch (obsolete) — Splinter Review
Attachment #8843020 - Attachment is obsolete: true
Attachment #8843020 - Flags: review?(jdescottes)
Attachment #8843050 - Flags: review?(jdescottes)
No longer blocks: 1326837
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.
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+
https://hg.mozilla.org/mozilla-central/rev/8e76e3bf6ce3
https://hg.mozilla.org/mozilla-central/rev/0598ed8d643b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Blocks: 1326837
Blocks: 1343054
Depends on: 1333602
Blocks: 1164853
Depends on: 1403011
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.