Closed Bug 307770 Opened 16 years ago Closed 14 years ago

Can't install XPI with a click on the link with target="_blank"

Categories

(Firefox :: Security, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 3 alpha8

People

(Reporter: vincent-moz, Assigned: Gavin)

References

()

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

I wanted to install <http://tmp.gary.elixant.com/beta/tabmix.xpi> from the above
URL. But each time I click on the link, a blank tab just appears (with no errors).

When I pasted the URL into the URL bar, the XPI could be installed without any
problem.

Reproducible: Always

Steps to Reproduce:
1. Open the above URL (the forum one).
2. Click on the link that is shown near the top.

Actual Results:  
A blank tab appears.

Expected Results:  
The XPI install dialog box should have appeared.
Summary: Can't install XPI with a click on the link (blank tab) → Can't install XPI with a click on the link (blankaa tab)
Summary: Can't install XPI with a click on the link (blankaa tab) → Can't install XPI with a click on the link (blank tab)
JS console says:

Error: [Exception... "Component returned failure code: 0x80004005
(NS_ERROR_FAILURE) [nsIURI.host]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)" 
location: "JS frame :: chrome://browser/content/browser.js :: anonymous :: line
457"  data: no]
Source File: chrome://browser/content/browser.js
Line: 457

where 457 is

var host =
browser.docShell.QueryInterface(Components.interfaces.nsIWebNavigation).currentURI.host;
Status: UNCONFIRMED → NEW
Component: Extension/Theme Manager → Security
Ever confirmed: true
OS: MacOS X → All
QA Contact: extension.manager → firefox
Hardware: Macintosh → All
Meant to resummarize: on a site that's not in the XPInstall whitelist, a link to
an XPI that opens a new window with target="_blank" correctly doesn't throw up
the install prompt, but also silently fails to throw up the blocked install
infobar, because it tries to get the host for the linking site from the current
docShell, which doesn't even have the incorrect host for the XPI, much less the
linking site.
Seems to me like the front end code needs to be able told which host triggered
the install without having to rely on the currentURI, which as shown is not
reliable (about:blank in the new window/tab case), since
nsInstallTrigger::HandleContent cancels the load and handles it itself.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpinstall/src/nsInstallTrigger.cpp&rev=1.74&mark=180#178
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/browser.js&rev=1.500&mark=515#511

I thought that using the data param
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpinstall/src/nsInstallTrigger.cpp&rev=1.74&mark=262#249
might be a good way of doing that, since that data is currently ignored by the
front end (it appears to have been reserved for differentiating between themes
and extensions, but never fully implemented)
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/browser.js&rev=1.500&mark=519-521#511

I really don't know whether that's a sane idea or not. I'd imagine it passing
the host of either |uri| or |referringURI|, depending on which was used.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpinstall/src/nsInstallTrigger.cpp&rev=1.74&mark=209,245#199
*** Bug 317931 has been marked as a duplicate of this bug. ***
Attached patch possible patch (obsolete) — Splinter Review
This patch:
1) Makes nsInstallTrigger::HandleContent pass the host that was checked to the "xpinstall-install-blocked" observer.

2) Makes the observer use that passed in host instead of trying to get it from currentURI

3) Introduces an aData param to <browsermessage> to allow passing arbitrary data (in this case the host) to it's observer

This kind of leads to confusing UI in the case of <a href="xpi" target="_blank">. A new blank window appears with the popup bar saying "this site ...", which could be misleading, and while it then allows you to add the site, you have to switch back to the other window and click again for it to install. I think this is better than the current behavior, but it's not optimal. I can't think of any optimizations, though, since installTrigger can only kick in once the load is started and the content-type is determined.

Thoughts?
Oh, and as I mentioned in comment 3, the aData param was previously meant to distinguish themes from extensions, apparently, but that was never implemented and the observers basically ignored it (see the removed XXXben comment). I figured it could just be removed.
(this patch fixes the two test cases in bug 317931 comment 8)
Note there are three more notifiers of "xpinstall-install-blocked".
This bug still exists in current Firefox 2.0 builds.

Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1) Gecko/20060915 BonEcho/2.0


Fehler: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.host]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js :: anonymous :: line 528"  data: no]
Quelldatei: chrome://browser/content/browser.js
Zeile: 528
Flags: blocking-firefox2?
Assignee: nobody → gavin.sharp
Flags: blocking-firefox2?
Version: unspecified → Trunk
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 3 alpha1
*** Bug 354359 has been marked as a duplicate of this bug. ***
Priority: -- → P2
Summary: Can't install XPI with a click on the link (blank tab) → Can't install XPI with a click on the link with target="_blank"
Target Milestone: Firefox 3 alpha1 → Firefox 3 beta1
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Attached patch updated patchSplinter Review
(In reply to comment #8)
> Note there are three more notifiers of "xpinstall-install-blocked".

I don't think those are relevant - they're the "native" xpinstall functions used from install.js, I think.
Attachment #204313 - Attachment is obsolete: true
Attachment #274467 - Flags: superreview?(dveditz)
Attachment #274467 - Flags: review?(mano)
Whiteboard: [patch-r?]
Comment on attachment 274467 [details] [diff] [review]
updated patch

sr=dveditz
Attachment #274467 - Flags: superreview?(dveditz) → superreview+
Keywords: checkin-needed
Whiteboard: [patch-r?]
mozilla/browser/base/content/browser.js 	1.823
mozilla/xpinstall/src/nsInstallTrigger.cpp 	1.79
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Depends on: 392885
(In reply to comment #11)
> (In reply to comment #8)
> > Note there are three more notifiers of "xpinstall-install-blocked".
> 
> I don't think those are relevant - they're the "native" xpinstall functions
> used from install.js, I think.

Bah, turns out they are :( See bug 392885.
Depends on: 393309
No longer depends on: 394254
You need to log in before you can comment on or make changes to this bug.