1400197 - js debugger broken on BSD

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Landry Breuil (:gaston)]

Opening the debugger in devtools on OpenBSD triggers lots of vomit:

console.error:                                                                                                               
  Message: TypeError: KEYS[os] is undefined                                                                                      
  Stack:                                                                                                               
    getKeyForOS@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/debugger/new/debugger.js:29495:4
getKey@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/debugger/new/debugger.js:29491:11

Of course, no match for Tier3 platforms in this... :

var KEYS = {
  WINNT: {
    resume: "F8",
    pause: "F8",
    stepOver: "F10",
    stepIn: "F11",
    stepOut: "Shift+F11"
  },
  Darwin: {
    resume: "Cmd+\\",
    pause: "Cmd+\\",
    stepOver: "Cmd+'",
    stepIn: "Cmd+;",
    stepOut: "Cmd+Shift+:",
    stepOutDisplay: "Cmd+Shift+;"
  },
  Linux: {
    resume: "F8",
    pause: "F8",
    stepOver: "F10",
    stepIn: "Ctrl+F11",
    stepOut: "Ctrl+Shift+F11"
  }
};

Please, think of the kittens and take into account the BSDs or other non-Tier1 platforms (solaris ?)

Comment 1

[Landry Breuil (:gaston)]

And this is since https://github.com/devtools-html/debugger.html/commit/d928662df130176ba02a3e1bcc94fdfe3aaf02cf - jason, which is the better way to fix this horror ?

Comment 2

[Jason Laster [:jlast]]

I'm sorry :gaston, I'll put a fix together today here: https://github.com/devtools-html/debugger.html/issues/4088

Is it okay to fall back on Linux shortcuts if the OS does not match?

Comment 3

[Henri Kemppainen]

Nobody asked me but yes, I would definitely expect to get Linux shortcuts on the BSDs.  And I suppose that's the most likely OS family when Firefox isn't running on one of the big three?

Comment 4

[Landry Breuil (:gaston)]

Yes, in lots of cases you can fallback to 'linux' defaults.

Comment 5

[Landry Breuil (:gaston)]

Fwiw, i tried hotpatching devtools/client/debugger/new/debugger.js in ffx 56.0rc1 source tarball

-         Linux: {
+         OpenBSD: {

but after a rebuild my patch/fix wasnt applied, i was still getting the same error, so i dunno what value appinfo.OS has on OpenBSD, and im not even sure *which* file ends up being used at runtime... even if i know its a bit late, would be good to fix that in 56.

Comment 6

[Landry Breuil (:gaston)]

Using this temporary workaround:


        function getKeyForOS(os, action) {
-         return KEYS[os][action];
+         return KEYS["Linux"][action];
        }

The debugger window in devtools seems to be working again. I don't know how you want to properly fix it though... maybe 

if (typeof KEYS[os] === 'undefined') {
    return KEYS["Linux"][action];
}
return KEYS[os][action];

instead of listing all possible unix-like operating systems (bsds, solaris, etc..)

Comment 7

[Tom Tromey :tromey]

One option might be to use widgetToolkit instead.
https://dxr.mozilla.org/mozilla-central/rev/ffe6cc09ccf38cca6f0e727837bbc6cb722d1e71/xpcom/system/nsIXULRuntime.idl#64-68
This runs into a similar problem, arguably, but the set of possible window systems is smaller, maybe just:
https://dxr.mozilla.org/mozilla-central/rev/ffe6cc09ccf38cca6f0e727837bbc6cb722d1e71/layout/tools/reftest/reftest.jsm#729-735

Comment 8

[Landry Breuil (:gaston)]

Yes, that would also work for us.

Comment 9

[Jason Laster [:jlast]]

This is now fixed on nightly.

Comment 10

[Landry Breuil (:gaston)]

What's needed to get this uplifted to beta/57, since there's no link to a m-c commit ? Just requiring approval-beta?

Comment 11

[Landry Breuil (:gaston)]

Also, it seems it was reverted on github: https://github.com/devtools-html/debugger.html/commit/3e16c37df19cca00e7f29842c622f5e678ee9ac4

Comment 12

[Landry Breuil (:gaston)]

was/would be a candidate to be?

Comment 13

[Landry Breuil (:gaston)]

Qk, so it was commited to m-c on https://hg.mozilla.org/mozilla-central/rev/a001ffb41787#l1.209 - but not yet reverted ?

Comment 14

[Landry Breuil (:gaston)]

The tracking flags are still wrong, and given the github<->m-c mess, i cant figure out if it was actually commited and it sticked into the tree, and in which branch... jason ?

Comment 15

[Jason Laster [:jlast]]

I believe the fix landed:

https://searchfox.org/mozilla-central/source/devtools/client/debugger/new/debugger.js#41643

and it came from this commit

https://searchfox.org/mozilla-central/diff/ebfefa4a4c1115c9f00bf9f0adb7fe87dac4646c/devtools/client/debugger/new/debugger.js#43657

Sorry for the large file sizes