Closed Bug 1323219 Opened 3 years ago Closed 3 years ago

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

Categories

(SeaMonkey :: Tabbed Browser, defect)

SeaMonkey 2.50 Branch
defect
Not set

Tracking

(seamonkey2.50 fixed)

RESOLVED FIXED
seamonkey2.50
Tracking Status
seamonkey2.50 --- fixed

People

(Reporter: philip.chee, Assigned: philip.chee)

References

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 file, 1 obsolete file)

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
Attachment #8818307 - Flags: review?(frgrahl)
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
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.50
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?
(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?
Then I do not understand the point. Why is Array.unshift() bad but Array.prototype.unshift() OK?
(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)
(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)?
(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)
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)
(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.