309265 - addSearchEngine fails silently if older version of a plugin already installed

Status: RESOLVED WORKSFORME

(Firefox :: Search, defect)

Description

[Joachim Noreiko]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-GB; rv:1.7.10) Gecko/20050717 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-GB; rv:1.7.10) Gecko/20050717 Firefox/1.0.6

The serachplugin repository, http://mycroft.mozdev.org/ lets you add plugins by
clicking on a javascript link, which calls window.sidebar.addSearchEngine to add
a plugin.

If an older version of a searchplugin already exists, the .src file is not
replaced with the new one. However, the user is not made aware of this. It
appears to the user that the new plugin has been installed successfully.

This bug makes it hard to update broken plugins that have beeen fixed at Mycroft.

Reproducible: Always



Expected Results:  
Ideally, asked what to do.
A warning that the plugin can't be installed that way & must be added manually
would do for now.

Comment 1

[Mycroft Project (CC)]

I looked into this problem already... I'll post some line numbers that explain why this happens if it'll save others from doing the same searching - I'm not quite sure what the ideal behaviour would be though.

Comment 2

[Mycroft Project (CC)]

Ok, this behaviour was introduced by bug 290038.
The issue is at:
http://lxr.mozilla.org/mozilla/source/xpfe/components/search/src/nsInternetSearchService.cpp#2628
which returns NS_ERROR_UNEXPECTED if the file already exists and it's not an internal update.
I'm not sure but it looks as though you could call AddSearchEngineInternal from a webpage and it would hack around this issue... but I'm not quite sure what you would pass as aOldEngineResource or if anything other than nsnull would do??
http://lxr.mozilla.org/mozilla/source/xpfe/components/search/src/nsInternetSearchService.cpp#2446

The solution IMO would appear to be an edit around line 2628 that asks of you want to overwrite the existing plugin or not - if so it continues, if not it breaks.

Unrelated, I'm not convinced the block you've added is entirely appropriate as that bug seems to refer to linux permissions and write failures only. (meta) Bug 171593 would appear better? (I haven't made that clear - my point is that 124224 could be fixed separately from this bug)

A final point is that plugins should be updated automatically since I fixed the update mechanism at mycroft but it does take x days depending on updateCheckDays and this can cause some problems where a user dls a plugin, the plugin is then updated (and is confirmed working) and the user trys again but doesn't realise it is necessary to remove the old before trying to install the new and says that the new is broken too! (If you see what I mean)

Comment 3

[O. Atsushi (Torisugari)]

(In reply to comment #2)
> The solution IMO would appear to be an edit around line 2628 that asks of you
> want to overwrite the existing plugin or not - if so it continues, if not it
> breaks.

Agreed. But we can catch the error and notify the user that he/she's just
failed to install the plugin. IMHO, the problem is not that the user
sometimes fails to install a plugin, but that the failure makes no noise.

Comment 4

[Mycroft Project (CC)]

(In reply to comment #3)
> Agreed. But we can catch the error and notify the user that he/she's just
> failed to install the plugin. IMHO, the problem is not that the user
> sometimes fails to install a plugin, but that the failure makes no noise.

I'm confused - you say you agree but then the next line doesn't seem to be agreeing entirely (I may have misunderstood though)

Notifying the user of the failure, while better than the current situation would not actually fix the problem.
To resolve the issue it is necessary to give the user the option to overwrite the plugin if that was what they were trying to do in the first place - I frequently have to delete files so that I can test an update that I have just made at mycroft.

Anyway, I don't have the ability to make the required changes but I would def/ like this bug to be sorted.

Comment 5

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

Is this bug still relevant to the new search service? If I remember correctly, installing a plugin when a plugin with the same name is already installed will always fail now.

Comment 6

[Mycroft Project (CC)]

It's certainly less relevant...
I would personally quite like to have the choice when adding a plugin where one of the same name already exists to a) overwrite or b) cancel or something.
It's usually when I'm trying to force an update so probably isn't typical usage but I think it would be more intuitive than removing and then readding.

It's obviously not critical though at this point and is somewhat outside the scope of this bug according to the title (though is mentioned in comment #1)

Comment 7

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

OK. I agree that we should handle conflicts better than we do now, and I don't think I've seen a bug filed about that specifically, so one should definitely be filed. This bug as summarized is now WORKSFORME, though, so I'll close this accordingly.