SebastianSimon / firefox-omni-tweaks

A script that disables the clickSelectsAll behavior of Firefox, and more.
MIT License
43 stars 8 forks source link

DevTools lack their tab header, possibly other L10n-related issues, caused by tampering with `browser/omni.ja` #2

Closed SebastianSimon closed 4 years ago

SebastianSimon commented 4 years ago

Note: DevTools lacking their tab header is only the first issue I noticed; there may be more. Ideally, this issue entry eventually captures all related issues. So far, all stack-traces end in null being passed to functionkey. Usually such calls look like functionkey(l10n( some string )). I currently assume that any symptomatic issue, whose culprit are these function calls, may have the same fix.

Recently, an issue has occured when you re-zip and replace browser/omni.ja (with or without any of the two sed changes):

Developer Tools lack their tab header

Shortly before or on 2020-07-09, any Firefox that has been tampered with lacks the tab header in the developer tools, i.e. the tabs to switch to “Inspector”, “Console”, “Debugger”, etc., picking an element, and the close button.

In the browser console, each time when opening the developer tools, these five errors are thrown in a row:

TypeError: can't access property "split", shortkey is null at react-dom.js:12769:13
    functionkey resource://devtools/client/definitions.js:669
    get tooltip resource://devtools/client/definitions.js:240
    render resource://devtools/client/framework/components/ToolboxTab.js:58
    React (13)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
    React (19)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
TypeError: can't access property "split", shortkey is null at react-dom.js:12769:13
    functionkey resource://devtools/client/definitions.js:669
    get tooltip resource://devtools/client/definitions.js:379
    render resource://devtools/client/framework/components/ToolboxTab.js:58
    React (13)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
    React (19)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
TypeError: can't access property "split", shortkey is null at react-dom.js:12769:13
    functionkey resource://devtools/client/definitions.js:669
    get tooltip resource://devtools/client/definitions.js:263
    render resource://devtools/client/framework/components/ToolboxTab.js:58
    React (13)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
    React (19)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
TypeError: can't access property "split", shortkey is null at react-dom.js:12769:13
    functionkey resource://devtools/client/definitions.js:669
    get tooltip resource://devtools/client/definitions.js:436
    render resource://devtools/client/framework/components/ToolboxTab.js:58
    React (13)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
    React (19)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    open resource://devtools/client/framework/toolbox.js:924
TypeError: can't access property "split", shortkey is null at definitions.js:669:3
    functionkey resource://devtools/client/definitions.js:669
    get tooltip resource://devtools/client/definitions.js:240
    render resource://devtools/client/framework/components/ToolboxTab.js:58
    React (13)
    setCanRender resource://devtools/client/framework/components/ToolboxController.js:125
    setCanRender self-hosted:935
    open resource://devtools/client/framework/toolbox.js:924
SebastianSimon commented 4 years ago

Developer Tools lack their tab header

I have analyzed this issue. It seems that l10n("styleeditor.commandkey") (and possibly other calls) within chrome/devtools/modules/devtools/client/definitions.js are null if tampered. This causes these tooltips to not be able to resolve to their intended text:

Tooltip over the “Debugger” tab says “JavaScript-Debugger (Ctrl+Shift+Z)”

The errors appear in this function:

function functionkey(shortkey) {
  return shortkey.split("_")[1];
}

which fails if shortkey is null. A simple (and safe) workaround is to replace the function by

function functionkey(shortkey) {
  return (shortkey || "_").split("_")[1];
}

This way, the tab bar renders properly, with the tooltips containing null strings or missing part of their text, e.g. “JavaScript Debugger (Ctrl+Shift+null)” or “Web Storage Inspector (Cookies, Local Storage, …) (Shift+)”. The shortcuts themselves still work. This is an acceptable compromise.

An updated version of the script will be uploaded shortly. Before closing this, I’ll be monitoring this issue, look out for related bugs, look for better alternatives, and see if this should (or will) be fixed by Mozilla instead.


It seems that this works without an issue for a few builds, e.g. 2020-08-18 and at least one earlier build.

If it does fail and if the final sed line is removed, I’ve noticed a couple other things:

Depending on your locale, chrome/⟨yourLocale⟩/locale/⟨yourLocale⟩/devtools/startup/key-shortcuts.properties is a very interesting file. Remember the l10n("styleeditor.commandkey") call? That’s where properties such as styleeditor.commandkey appear to be defined. There’s three peculiarities:

SebastianSimon commented 4 years ago

Looks like I’ve found the answer: this semi-related Super User answer says

The resulting file is however about 2.5 times smaller than the original so I assume firefox doesn't use 9 as compression level these days but 0. Indeed

zip -qr0XD omni.ja *

produces a file almost the exact same size as the original.

I never cared about the file size of omni.ja. Yes, I always found it peculiar how the size after repacking was only a fraction of the original, but it seemed to work, and it was documented as such on MDN (archived page), clearly stating (emphasis mine):

[…]; editing extracted files won't affect Firefox, and repacking edited files may break Firefox if you do not use the right options when packing the extracted files. The correct command to pack omni.ja is:

zip -qr9XD omni.ja *

After several failed attempts, back when I first attempted to tamper with this file, I really believed it. I really believed that without these exact options (well, except -q), there’s no chance to make any edit work.

Now, not only is the file size nearly identical to the original omni.ja, but the tab header appears to fully work now! :tada: Also, the script runs a lot faster!


Next steps:

  1. [x] Test the script with the zip -0 option instead of the -9 option for a few days. Maybe this is all a coincidence or a dream! It appears to work with a build on 2020-08-19 and two builds on 2020-08-20.
  2. [x] Test different compression levels and pick the highest that still works. -9 may actually be an oddball1, so I’ll test with -8 right away; I’d be surprised if that doesn’t work. Edit after trying: Well, color me surprised: only -0 works.
  3. [x] Update the script here.
  4. [x] Update the Super User answer.
  5. [x] Document the correct compression level(s) to use on MDN.

1 The zip man page states (emphasis mine):

-# (-0, -1, -2, -3, -4, -5, -6, -7, -8, -9)

Regulate the speed of compression using the specified digit #, where -0 indicates no compression (store all files), -1 indicates the fastest compression speed (less compression) and -9 indicates the slowest compression speed (optimal compression, ignores the suffix list). […]

So -9 ignores the file suffix? I thought that’s what made repacking omni.ja work in the first place, but it may be the thing that actually breaks it.