1323219 - Fix tabbrowser error from removal of javascript Array generics (Bug 1322124)

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Philip Chee]

Timestamp: 12/13/2016 4:54:45 PM
Error: TypeError: aArguments.unshift is not a function
Source File: chrome://navigator/content/tabbrowser.xml
Line: 419
Timestamp: 12/13/2016 4:54:45 PM
Error: TypeError: aArguments is undefined
Source File: chrome://navigator/content/tabbrowser.xml
Line: 419

Comment 1

[Philip Chee]

Attachment: Patch v1.0 fix Error: TypeError: aArguments.unshift is not a function

Comment 2

[Philip Chee]

Attachment: Patch v1.1 Add fix to messengerdnd.js

This patch Adds fix to messengerdnd.js

Comment 3

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8818311 [details] [diff] [review]
Patch v1.1 Add fix to messengerdnd.js

r=me a=you

Comment 4

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/comm-central/rev/5c3c8ec153ac5aefea869deeaafa110e05f04adc

Comment 5

[:aceman]

How does the fix work? Where is aAraguments in the changed code? Also, is this putting back Array.unshift, that was intended to be removed? Why can't it be converted to something else here?

Comment 6

[Philip Chee]

(In reply to :aceman from comment #5)
> How does the fix work?
I don't know. Maybe Array.from() does something different?

> Where is aAraguments in the changed code?
Error: TypeError: aArguments.unshift is not a function
Source File: chrome://navigator/content/tabbrowser.xml
Line: 419

> Also, is this putting back Array.unshift, that was intended to be removed?
No this is using Array.prototype.unshift which always works.

> Why can't it be converted to something else here?
Um, this works. What should it be converted to?

Comment 7

[:aceman]

Then I do not understand the point. Why is Array.unshift() bad but Array.prototype.unshift() OK?

Comment 8

[Philip Chee]

(In reply to :aceman from comment #7)
> Then I do not understand the point.
> Why is Array.unshift() bad but
Array generics e.g. Array.unshift is not on the standards track.

> Array.prototype.unshift() OK?
Array.prototype.unshift() is in the standard since forever.

General discussion:
The raison d'être for the existence of Array Generics is to avoid the ugliness of e.g.
Array.prototype.unshift.call(arguments, this.mBrowser)
With Generics you can then use:
Array.unshift(arguments, this.mBrowser);

Comment 9

[:aceman]

(In reply to Philip Chee from comment #8)
> (In reply to :aceman from comment #7)
> > Then I do not understand the point.
> > Why is Array.unshift() bad but
> Array generics e.g. Array.unshift is not on the standards track.
> 
> > Array.prototype.unshift() OK?
> Array.prototype.unshift() is in the standard since forever.

But what is the difference between the two?

> With Generics you can then use:
> Array.unshift(arguments, this.mBrowser);

And what is the difference to arguments.unshift(this.mBrowser) (assuming arguments is an array)?

Comment 10

[Philip Chee]

(In reply to :aceman from comment #9)
> (In reply to Philip Chee from comment #8)
> > (In reply to :aceman from comment #7)
> > > Then I do not understand the point.
> > > Why is Array.unshift() bad but
> > Array generics e.g. Array.unshift is not on the standards track.
> > 
> > > Array.prototype.unshift() OK?
> > Array.prototype.unshift() is in the standard since forever.
> 
> But what is the difference between the two?
> 
> > With Generics you can then use:
> > Array.unshift(arguments, this.mBrowser);
> 
> And what is the difference to arguments.unshift(this.mBrowser) (assuming
> arguments is an array)?

Maybe you can ask arai. He's the JavaScript expert!

Comment 11

[Tooru Fujisawa [:arai]]

(In reply to Philip Chee from comment #6)
> (In reply to :aceman from comment #5)
> > How does the fix work?
> I don't know. Maybe Array.from() does something different?

Array.from(X) returns newly created Array, so modifying it doesn't change X.
If you want to put an element in front of `arguments`, you should operate directly onto `arguments`, not newly created array.

if you want to use Array.from(arguments), you need to assign it to other variable and pass it instead of `arguments`, when calling apply:

    let tmp = Array.from(arguments);
    tmp.unshift(this.mBrowser);
    return this.mTabBrowser._callProgressListeners.apply(this.mTabBrowser, tmp);


(In reply to :aceman from comment #9)
> But what is the difference between the two?

Array.unshift(X, Y) does same thing as Array.prototype.unshift.call(X, Y)
(the internal implementation does Array.prototype.unshift.apply(X, [Y]), without accessing property)


> > With Generics you can then use:
> > Array.unshift(arguments, this.mBrowser);
> 
> And what is the difference to arguments.unshift(this.mBrowser) (assuming
> arguments is an array)?

`arguments` is not an Array, and it doesn't have unshift property.

Comment 12

[:aceman]

(In reply to Tooru Fujisawa [:arai] from comment #11)
> (In reply to :aceman from comment #9)
> > But what is the difference between the two?
> 
> Array.unshift(X, Y) does same thing as Array.prototype.unshift.call(X, Y)
> (the internal implementation does Array.prototype.unshift.apply(X, [Y]),
> without accessing property)

So why does have one to go away when they are the same? Just reducing the API (calling methods), but not removing functionality?

Comment 13

[Tooru Fujisawa [:arai]]

(In reply to :aceman from comment #12)
> So why does have one to go away when they are the same? Just reducing the
> API (calling methods), but not removing functionality?

Since Array.unshift is non-standard SpiderMonkey-only extension and we're going to deprecate it (and also other Array generics)