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

RESOLVED FIXED in seamonkey2.50

Status

SeaMonkey
Tabbed Browser
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

SeaMonkey 2.50 Branch
seamonkey2.50

SeaMonkey Tracking Flags

(seamonkey2.50 fixed)

Details

User Story

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);

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
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
(Assignee)

Comment 1

a year ago
Created attachment 8818307 [details] [diff] [review]
Patch v1.0 fix Error: TypeError: aArguments.unshift is not a function
Attachment #8818307 - Flags: review?(frgrahl)
(Assignee)

Comment 2

a year ago
Created attachment 8818311 [details] [diff] [review]
Patch v1.1 Add fix to messengerdnd.js

This patch Adds fix to messengerdnd.js
Attachment #8818307 - Attachment is obsolete: true
Attachment #8818307 - Flags: review?(frgrahl)
Attachment #8818311 - Flags: review?(frgrahl)
Comment on attachment 8818311 [details] [diff] [review]
Patch v1.1 Add fix to messengerdnd.js

r=me a=you
Attachment #8818311 - Flags: review?(frgrahl) → review+
https://hg.mozilla.org/comm-central/rev/5c3c8ec153ac5aefea869deeaafa110e05f04adc
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-seamonkey2.50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.50

Comment 5

a year ago
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?
(Assignee)

Comment 6

a year ago
(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

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

Comment 8

a year ago
(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);
User Story: (updated)

Comment 9

a year ago
(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)?
(Assignee)

Comment 10

a year ago
(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!
Flags: needinfo?(acelists)

Updated

a year ago
Flags: needinfo?(acelists) → needinfo?(arai.unmht)
(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.
Flags: needinfo?(arai.unmht)

Comment 12

a year ago
(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?
(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)
You need to log in before you can comment on or make changes to this bug.