1044108 - QR decoder needs cleanup and size reduction

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

The QR decoder file wastes lots of space with comments and massive whitespace.  Let's at least trim it and possibly minify.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Part 1: Beautify QR decoder, keep only Mozilla and license comments

Paul, this change is a one-time pass to reformat the QR decoder with a JS beautifier.  There is no real need to keep files in the style of the upstream project, as it doesn't have much activity.

This beautified file will be what we'd edit if we need to tweak something else in the future.  I've killed all comments, except the license headers and Mozilla specific notes.

The part takes us from 114kB to 90kB.  Also, the file is now possible to be read by humans.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Part 2: Minify QR decoder with Uglify

In this step, I've moved the human-readable version to |original.js|, and replaced |index.js| with a minified version.  The new README contains the Uglify command I used for future reference.  The decoder code is basically a black box for us, so I think it's fine to minify like this and ship that version.

This part takes us from 90kB to 46kB, so we're now at less the half the starting size.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Part 3: Disable QR decoder on Fennec for now

Paul, to support Fennec's goal of removing toolkit bits they don't need (bug 1044079), I've changed things to not ship it for that application.

gps, can you review the build changes here?

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=d3ec8d613565

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8463529 [details] [diff] [review]
Part 3: Disable QR decoder on Fennec for now

Review of attachment 8463529 [details] [diff] [review]:
-----------------------------------------------------------------

Build bits are fine.

Comment 6

[Paul Rouget [:paul]]

(In reply to J. Ryan Stinnett [:jryans] from comment #2)
> Created attachment 8463525 [details] [diff] [review]
> Part 2: Minify QR decoder with Uglify
> 
> In this step, I've moved the human-readable version to |original.js|, and
> replaced |index.js| with a minified version.  The new README contains the
> Uglify command I used for future reference.  The decoder code is basically a
> black box for us, so I think it's fine to minify like this and ship that
> version.
> 
> This part takes us from 90kB to 46kB, so we're now at less the half the
> starting size.

Why do we minify this file specifically? What about all the other JS files we ship in B2G and Fennec?

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #6)
> (In reply to J. Ryan Stinnett [:jryans] from comment #2)
> > Created attachment 8463525 [details] [diff] [review]
> > Part 2: Minify QR decoder with Uglify
> > 
> > In this step, I've moved the human-readable version to |original.js|, and
> > replaced |index.js| with a minified version.  The new README contains the
> > Uglify command I used for future reference.  The decoder code is basically a
> > black box for us, so I think it's fine to minify like this and ship that
> > version.
> > 
> > This part takes us from 90kB to 46kB, so we're now at less the half the
> > starting size.
> 
> Why do we minify this file specifically? What about all the other JS files
> we ship in B2G and Fennec?

Mainly, I was trying to be nice to applications that don't currently minify globally (I believe b2g is the only one minifying globally currently), since I know that this file in particular would not be hurt too much by being minified, since it's external code we're using.

In any case, I don't feel too strongly about this, so we can also wait for Fennec to enable global chrome minification as part of size reduction (assuming they decide to do so).

Even without this, parts 1 and 3 still seem like good things to do.

Comment 8

[Paul Rouget [:paul]]

Comment on attachment 8463525 [details] [diff] [review]
Part 2: Minify QR decoder with Uglify

I'm not sure we want to ship cryptic code. If we want to minify this code, let's do it from the makefile.

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Part 1: Beautify QR decoder, keep only Mozilla and license comments (v2, paul: r+)

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Part 2: Disable QR decoder on Fennec for now (v2, paul: r+, gps: r+)

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=794a622e3efd

Comment 12

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/c1854c559c4f
https://hg.mozilla.org/integration/fx-team/rev/5c8555e5adfe

Comment 13

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/5c8555e5adfe
https://hg.mozilla.org/mozilla-central/rev/c1854c559c4f

Comment 14

[Richard Newman [:rnewman]]

\o/