When we added whitelisting to xpinstall launches it seemed very few sites bothered to check InstallTrigger.enabled(), and that those that did would prefer knowing the install was likely to fail so they could give a message to their users. It may be better to return enabled() to its previous value (the global install state, generally "true") and let the site attempt to install and then let the whitelisting block lead to the user-visible info-banner.
> ... would prefer knowing the install was likely to fail so they could give a > message to their users. I believe that this is still somewhat true. But, sites would only expect InstallTrigger.enabled() to return false if the user took some action to prevent software update. Hence, the message a site might show the user would be designed for a user who would understand what they did to disable software update in the first place. Moreover, a webapp that pre-dates XPI whitelisting in Mozilla would be unable to explain XPI whitelisting to the user! Hence, the burden is on the browser to explain XPI whitelisting to the user. We cannot expect all webapps to be revised overnight, so we must rely on the browser to provide some useful feedback to the user when XPI whitelisting is the reason that an install failed. If reverting InstallTrigger.enabled() to the older behavior is the best way to do this, then I'm all for it.
Adding testcase. It turns out xulmine.mozdev.org was likewise broken.
Status: NEW → ASSIGNED
*** Bug 258641 has been marked as a duplicate of this bug. ***
after some discussion sounds like we want to release note this and/or review futher before PR.
saving the install page to a local file, then clicking on the install link works as a very ugly and difficult to discover work around...
Comment on attachment 158795 [details] [diff] [review] patch r=jst
Attachment #158795 - Flags: review+
Comment on attachment 158795 [details] [diff] [review] patch no good, this one disabled whitelisting entirely -- all the nsJSInstallTrigger entrypoints call through here. We'll have to put an explicit pref check into nsJSInstallTrigger instead, or pass some sort of boolean into this one (which might be useful to allow installChrome to bypass whitelisting as well).
Attachment #158795 - Flags: superreview-
Chofmann made me r+ it, really! :-)
decided to take this for PR... dan is testing an update to the patch.
Flags: blocking-aviary1.0PR? → blocking-aviary1.0PR+
Created attachment 158817 [details] [diff] [review] aviary patch Same basic idea, but pass a flag from the callsites whether they need global or whitelist checking. The reason for the flag, rather than just putting pref code directly into InstallTriggerGlobalUpdateEnabled() is that the flag can be used to solve bug 246375 at the same time. I've slipped that into this patch -- if you don't like it I can change the installChrome() call to simply pass XPI_WHITELIST without checking the chromeType and fix only this bug.
Attachment #158795 - Attachment is obsolete: true
Comment on attachment 158817 [details] [diff] [review] aviary patch Note this is a -w diff to make reviews easier. I'll apply to all branches. If someone else wants to apply it watch for whitespace issues.
Comment on attachment 158817 [details] [diff] [review] aviary patch firstname.lastname@example.org
Attachment #158817 - Flags: review?(bugs) → review+
Comment on attachment 158817 [details] [diff] [review] aviary patch sr=jst
Attachment #158817 - Flags: superreview?(jst) → superreview+
+ prefBranch->GetBoolPref( XPINSTALL_ENABLE_PREF, aReturn); doesn't look like the space after the '(' is the style of the file
Comment on attachment 158817 [details] [diff] [review] aviary patch a=chofmann for PR
Attachment #158817 - Flags: approval-aviary? → approval-aviary+
Fixed on trunk plus aviary and 1.7 branches
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Keywords: fixed-aviary1.0, fixed1.7.x
Resolution: --- → FIXED
This doesn't seem fixed to me in the latest round of respins available at http://ftp28f.newaol.com/pub/mozilla.org/firefox/nightly/2004-09-13-19-0.9/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It is and it isn't... the bug described here is fixed, but several sites are suffering from two bugs. Any site doing: if (InstallTrigger.enabled()) InstallTrigger.install(....); //or installChrome() is now fixed by this patch. Any site doing if (InstallTrigger.enabled()) InstallTrigger.startSoftwareUpdate(...); is /closer/ to fixed, but still doesn't work. The startSoftwareUpdate function does not send the notifications that the UI uses to put up the infobar.
Just to keep problems straight--the bug as narrowly defined here IS fixed--I filed bug 259211 on the startSoftwareUpdate problem. The xulmine.mozdev.org link above suffers from both problems, but other sites are fixed by just this bug.
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago → 14 years ago
No longer depends on: 259211
Resolution: --- → FIXED
*** Bug 252015 has been marked as a duplicate of this bug. ***
Attachment #158817 - Flags: approval1.7.x? → approval1.7.x+
You need to log in before you can comment on or make changes to this bug.