521065 - Overriding gBrowser.addTab doesn't work properly

Status: RESOLVED INVALID

(Firefox :: Tabbed Browser, defect)

Description

[Mike Kaply [:mkaply]]

Attachment: Extension that overrides addtab

Many extensions (especially ones that modify tab behavior) override gBrowser.addTab in order change things after a new tab is opened.

This no longer works when a tab is opened using the middle mouse button or right click, open Link in new tab.

The tab opens, but never loads.

The error is:

Error: [Exception... "'JavaScript component does not have a method named: "clone"' when calling method: [nsIURI::clone]"  nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)"  location: "JS frame :: chrome://global/content/bindings/browser.xml :: loadURIWithFlags :: line 186"  data: no]
Source File: chrome://global/content/bindings/browser.xml
Line: 186

I've attached an extension that replaces AddTab.

You can check the behavior on Firefox 3.5 and Firefox 3.6.

Comment 1

[Dão Gottwald [:dao]]

1. The proper way to change things after a new tab is opened is to listen to the TabOpen event.
2. The test extension calls gBrowser.origAddTab(a,b,c,d,e,f). Instead, it should do origAddTab.apply(gBrowser, arguments).

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Is there much of a benefit to the arguments.length check here? Perhaps we should just remove it...

Comment 3

[Mike Kaply [:mkaply]]

Doh. Forgot about the apply. That's my mistake.

But I do know one large set of extensions that will be broken by this because they don't use the .apply - every BenefitBar.

That's where I saw this done originally.

Comment 4

[Dão Gottwald [:dao]]

The arguments.length check makes sense because it's cheaper than instanceof Ci.nsIURI.

gBrowser.origAddTab(a,b,c,d,e,f) also assumes that we never add a seventh argument. It's really a harmful pattern.

Comment 5

[Mike Kaply [:mkaply]]

Actually it was Freecause and they seemed to have fixed it, so I'll take the INVALID.