typeof changed even when "unsafe" is false

[timmywil]

We ran into an issue when compressing jQuery with the latest version (3.0.23). The README implies that the typeof should remain unchanged when unsafe is set to false. Nevertheless, only when setting ie8 to true did we get the expected output.

Sometimes, methods are invoked in IE9/IE10 when simply referencing them as properties. In this case, .getAttribute is called despite the lack of parens, throwing an error. Only when typeof remains untouched is the error avoided. Reduced example is below.

Bug report
ES5
Uglify version (uglifyjs -V)
uglify-js 3.0.23

JavaScript input

function attr (elem, name, value) {
	// Fallback to prop when attributes are not supported
	if (typeof elem.getAttribute === "undefined") {
		return jQuery.prop(elem, name, value);
	}
}

The uglifyjs CLI command executed or minify() options used.

uglifyjs --compress -- test.js

or

uglifyjs --compress unsafe=false -- test.js

JavaScript output or error produced.

function attr(elem,name,value){if(void 0===elem.getAttribute)return jQuery.prop(elem,name,value)}

Correct output is produced with

uglifyjs --ie8 --compress -- test.js

Output being:

function attr(elem,name,value){if("undefined"==typeof elem.getAttribute)return jQuery.prop(elem,name,value)}

Problem is, the output is unsafe for IE9/IE10 as well.

[alexlamsl]

There is no such CLI option as --unsafe, only --compress unsafe

If I understand you correctly, are you implying typeof elem.getAttribute does not invoke getter somehow? I just tested this under IE Developer Tools:

> var e = {};
> Object.defineProperty(e, "prop", {get:function(){console.log("called")}});
> e.prop;
undefined
called

> typeof e.prop
"undefined"
called

> typeof e.prop === "undefined"
true
called

> e.prop === void 0
true
called

Possibly related to #1446

[timmywil]

@alexlamsl

There is no such CLI option as --unsafe, only --compress unsafe

Already edited. See above.

This is not related to property getters, only native methods like getAttribute. It's hard to reproduce, but jQuery has a certain XML test that falls back to properties that reproduces it. And yes, the method is not called if using typeof.

[timmywil]

We adopted the typeof style over if (!elem.getAttribute) a long time ago because of this issue. While I think it happened more often in IE6-8, it still happens under certain circumstances in IE9-10. It's obscure in that even those that know about this IE bug forget that it's there. People don't run into it that often, but jQuery still guards against it by using typeof.

[timmywil]

Actually, I was able to reproduce with a short test using XML: http://jsbin.com/hokafa

Open in IE9 or IE10.

You'll see "Wrong number of arguments" in the console.

[kzc]

I don't run Windows so I am unable to verify this.

I'm having difficulty understanding these statements:

Correct output is produced with

uglifyjs --ie8 --compress -- test.js

Problem is, the output is unsafe for IE9/IE10 as well.

Does the --ie8 flag produce code that will run correctly on IE8, IE9 and IE10? If that's the case, I'd recommend to use --ie8 as this IE javascript behavior is non-standard.

If the --ie8 flag does not work for IE8/IE9/IE10 another alternative is:

$ bin/uglifyjs 2198.js -c comparisons=false
function attr(elem,name,value){if("undefined"===typeof elem.getAttribute)return jQuery.prop(elem,name,value)}

[alexlamsl]

@timmywil thanks for the test case. I'll have a look later on today under various versions of IE.

Meamwhile, as @kzc pointed out, workaround involves either --compress comparsons=false or --compress ie8.

[timmywil]

@kzc @alexlamsl I know the workaround. I gave an example of the workaround. The bug is simply that the typeof transformation happens when unsafe is false, when the docs at least imply that it shouldn't by listing it among the unsafe transformations.

[timmywil]

It would be ideal for me if the transformation was dependent on unsafe being true, but the alternative is to document that some "unsafe" transformations can only be turned off with the --ie8 flag, which is really more of an "oldIE" flag in this case, but that's just a semantic issue.

[timmywil]

I just re-read @kzc's comment. I didn't realize this was included in comparisons. That does seem odd as well.

[kzc]

Despite the incorrect "unsafe" documentation, the typeof transform was deemed to be safe as of #1446. When that decision was made it was thought that only IE < 9 had that non-standard javascript behavior.

It would be unfortunate if this typeof transformation has to be disabled by default under the "unsafe" option due to IE9 and IE10 with their collective worldwide browser market share below 1 percent - but so be it. There are still bug workarounds for other browsers including ancient Safari versions that are enabled by default.

[timmywil]

That's fair. Although, it also seems unfortunate that jQuery has to be compressed with ie8 set to true when it doesn't actually support ie8.

[timmywil]

The upside is that even with ie8 set to true, our recent upgrade of uglify still reduced the size by 16 gzipped bytes. So, all in all, I think Uglify is doing an excellent job. Thanks.

[kzc]

-c comparisons=false ought to produce smaller code than --ie8 - without gzip anyway.

--ie8 has a lot of other baggage. It covers various IE8 bugs in mangle, catch scope, compress and output bugs (keyword properties and \v).

[alexlamsl]

@timmywil

You'll see "Wrong number of arguments" in the console.

Yeah, and I also get typeof elem.getAttribute === "unknown" - very odd.

My theory is that IE9/10 treats elem.getAttribute as elem.getAttribute(), i.e. auto-insert the call parenthesis. Why they think this is a good idea is beyond me.

TIL:

> elem.getAttribute()
Wrong number of arguments or invalid property assignment

> elem.getAttribute(0)
This name may not begin with the '0' character:

-->0<--

> elem.getAttribute(true)
This name may not begin with the '-' character:

-->-<--1

> elem.getAttribute(null)
Type mismatch

> elem.getAttribute("foo")
null

[kzc]

@alexlamsl In light of these findings, what's the best course of action?

1. Leave code as is and update unsafe documentation, and document IE9/IE10 typeof workarounds.
2. Have the typeof transform once again be governed by the compress option unsafe.
3. Create a new compress option specifically for the typeof transform defaulting to enabled.
4. Create a new compress option specifically for the typeof transform defaulting to disabled.

[alexlamsl]

I think (3) is probably the best way to go - I was thinking of introducing compress.ie10 👻

[kzc]

(3) is fine, but I'd suggest naming it after what it does - typeofs - plural to follow the convention of the other compress options.

Would also have to update the unsafe documentation in any case.

[alexlamsl]

Thank you - I really wasn't thrilled about ie10, truth be told 😅

Would also have to update the unsafe documentation in any case.

👍

[timmywil]

Thank you @kzc and @alexlamsl! You two rock.