1758568 - JS Minifier fails on Extension.jsm

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P3)

Description

[[:fabrice] Fabrice Desré]

The classifyOriginPermissions function at https://searchfox.org/mozilla-central/rev/9ca193b4233957439583f2eadabbd3cfb4cd9fed/toolkit/components/extensions/Extension.jsm#1662 triggers a bug in the minifier: here's the minified version:

static classifyOriginPermissions(origins=[]){let allUrls=null,wildcards=new Set(),sites=new Set();for(let permission of origins){if(permission=="<all_urls>"){allUrls=permission;break;}

let match=/^[a-z*]+:\/\/([^/]*)\/|^about:/.exec(permission);      if (!match) {        throw new Error(`Unparseable host permission ${permission}`);      }//
Note:the scheme is ignored in the permission warnings.If this ever

if(!match[1]||match[1]=="*"){allUrls=permission;}else if(match[1].startsWith("*.")){wildcards.add(match[1].slice(2));}else{sites.add(match[1]);}}
return{allUrls,wildcards,sites};}

This is causing a SyntaxError: unexpected token: identifier parsing error which was kind of hard to track in the long chain of promises / async functions and only appeared as a failure to download a corrupt extension...

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

Hmm, interesting. Can you directions to reproduce this issue?

Comment 2

[[:fabrice] Fabrice Desré]

(In reply to Mitchell Hentges [:mhentges] 🦀 from comment #1)

Hmm, interesting. Can you directions to reproduce this issue?

It looks like the js minifier is enabled on b2g gonk builds. My local fix is to add ac_add_options --enable-minify=properties in my mozconfig, so I guess you can repro with ac_add_options --enable-minify=properties,js

Comment 3

[Nick Alexander :nalexander [he/him]]

fabrice: are you saying that Bug 1752968 enabled JS minification in a place that it was not enabled before? If so, that wasn't intended: I tried pretty hard to keep the conditions identical.

Comment 4

[[:fabrice] Fabrice Desré]

Minification was not enabled before on Gonk builds, but that changed because of the enable_minify_default() rules. I can believe you that nothing changed for official Geckoview builds.

However that's the first time I see the minifier failing, and that's the real issue.

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to [:fabrice] Fabrice Desré from comment #4)

Minification was not enabled before on Gonk builds, but that changed because of the enable_minify_default() rules. I can believe you that nothing changed for official Geckoview builds.

Ah, you are correct: I changed a thing to is_android where before it was only for mobile/android. I'm surprised we don't see the same issue for GeckoView, but perhaps we are seeing it and we just aren't looking for it.

NI to me to get back to this.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1752968

Comment 7

[James Graham [:jgraham]]

AIUI this isn't an issue for Firefox, and as such we don't need to get or uplift a patch for 99?

Comment 8

[Pier Angelo Vendrame]

Hi,
I have noticed that an old version (2.1.0) of jsmin is versioned, and updating to the latest one solves this problem.

Comment 9

[[:fabrice] Fabrice Desré]

Does anyone know the process to update jsmin?

Comment 10

[Mike Hommey [:glandium]]

Update third_party/python/requirements.in and run mach vendor python. I think there are ongoing issues with mach vendor python, though.

Comment 11

[[:fabrice] Fabrice Desré]

Attachment: Bug 1758568 - Update jsmin to 3.0.0 r=glandium

Comment 12

[[:fabrice] Fabrice Desré]

What needs to happen to get that landed since glandium accepted the change in phabricator?

Comment 13

[Nick Alexander :nalexander [he/him]]

(In reply to [:fabrice] Fabrice Desré from comment #12)

What needs to happen to get that landed since glandium accepted the change in phabricator?

I landed it. You could also set checkin-needed, I think.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e2b1dfec1ab
Update jsmin to 3.0.0 r=glandium

Comment 15

[[:fabrice] Fabrice Desré]

Thanks Nick!

Comment 16

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5e2b1dfec1ab

Comment 17

[Takanori MATSUURA]

Just for information.

The latest version seems to be 3.0.1.
We do not have to update it because it only updates test.py.