Closed Bug 306576 Opened 15 years ago Closed 15 years ago
Search plugins URLs should be standardized on *secure* mozilla
500 bytes, patch
|Details | Diff | Splinter Review|
4.18 KB, patch
|Details | Diff | Splinter Review|
589 bytes, patch
|Details | Diff | Splinter Review|
4.16 KB, patch
|Details | Diff | Splinter Review|
Search plugin URLs, especially the localized search plugin URLs, are set to all kinds of bad things. I would rather we just didn't have any search plugin updates for our default searchplugins, but if we're going to have them, they should be a standardized URL on a secure mozilla.org server (not www.mo). Mconnor and I already discussed this, and for this cycle the URL should be: https://addons.mozilla.org/searchplugins/updates/<searchplugin>.src In addition, all the localized searchplugins should have unique names, i.e. instead of google-france searchplugin being named "google.src" it should be "google-fr.src".
When this bug is fixed, bug 284456 will disappear without any client-side hack.
(In reply to comment #0) > In addition, all the localized searchplugins should have unique names, i.e. > instead of google-france searchplugin being named "google.src" it should be > "google-fr.src". When Firefox 1.0 users install version 1.5, wouldn't they have duplicated search engines? e.g. "google.src" (from 1.0) + "google-fr.src" (from 1.5).
Won't it give us a list of shortcommings like 1) When mozilla.org is down, our users can't use searchengines 2) When mozilla.org is slow, searchplugins works slow etc.? I'm not sure what bad things did you mean? Maybe there's another way to fix them without subordinating searchplugins to mozilla.org servers.
I don't like this idea at all. Although you prevent bad l10n teams to set their localized search plugin URLs to all kinds of bad things, you also prevent the updating mechanism to work. By now the search plugins of Czech Firefox are hosted directly on the search engine provider (i.e. Seznam.cz plugin is on Seznam.cz server). So if the search engine is changed, the administrator can easily change the search plugin, and these changes are automaticaly propagated to users. If all search plugins are on a.m.o there will be no automatical update. The plugin will just stop working, users will complain, we (l10n team) have to download the new plugin from the server, fill a bug, nag on IRC to get someone with enouch rights to fix it ... So please, if we're going to have the search plugin updates, allow them to be also hosted on the server of origin.
"Bad things" include most google plugins referring to the english version (which is a 404, so nothing bad happens). On top of that, the search plugin is part of a Mozilla product, and changing that on the descretion of another entitity may sound easy, but from a product development point, this is a daunting thing. We did have problems with search engines before, and it's good to control the updates. Note that this is only about the update URLs, the functionality of the plugins is not affected by mozilla.org server uptimes.
The idea sounds good, but I would rather use "google.de" and not "google-de". It looks a lot more familiar for users.
I don't expect end-users will be seeing google-de.src vs google.de.src and getting confused, especially since this is just filenames. Sure, its slightly less convenient for individuals to have to go through an approvals process to get these uploaded. That said, we want to ensure that we control this aspect of our product for official builds, for the same reason that we're providing official builds ourselves. I seriously doubt that searchplugin updates are occuring so frequently that this is going to represent a major hardship for server operators. Or that things will change in a timeframe such that it is not possible to get new searchplugins to the server before the system change. As for getting new versions uploaded, I expect that this would be handled via bugzilla as a mozilla.org Server Ops change request, with the new searchplugin attached for transparency purposes. Not a lot of work to file a bug and attach a file.
(In reply to comment #7) > I don't expect end-users will be seeing google-de.src vs google.de.src and > getting confused, especially since this is just filenames. Sorry my bad, but what about comment #2? Having double entries for updaters would be pretty bad.
Yes, this is a problem since we can't do clean updates automagically because of people installing into weird places. I could be ok with punting on that naming convention for now since we want to make search "better" in 2.0, unless someone has a better solution.
Now that search plugins can be installed by extensions, I suggest: The update declarations are removed from the standard search plugins and each of them is packaged into an extension, which is hosted by mozilla.org (presumably on addons.m.o). The default search plugins are provided as default extensions to Firefox. Any updates are handled by the extension update system. This would give the ability to uninstall search plugins and the ability to disable their installation (bug 289826) for free. It would also ensure that the old src file is removed during an update before the new one is added, and that the icon is always available and updated properly (because it would be inside the XPInstaller).
Requesting beta 2 blocking, this has quite some fallout for l10n.
Pike, I'm on vacation next week, are you interested in taking/driving this?
taking over for now.
Assignee: benjamin → l10n
Could we get a sign-off of the URL scheme proposed by Benjamin in the initial comment from UMO server folks? I'd like to improve this situation in our 39+ locales soon. I'm following mconnor in that these filenames are not user-visible. I don't see a good way around the duplicate entries for users updating without not fixing bugs. Having that work as it should is subject of bug 232272 or bug 235852, which according to gavin is due to 2.0.
Really, giving users double entries in their search plugin list, without a user iterface to delete them (which, by the, way is one of the most stupid things I've ever seen) should be a no-go.
Is there a reason why the URL should not be: https://addons.mozilla.org/searchplugins/updates/ab-CD/<searchplugin>.src ? -------------------------------------------------^^^^^ no need to rename files, no duplicate entries.
yes, having wikipedia.src as file name for all wikipedia plugins makes it impossible to have more than one wikipedia plugin installed. That's why this naming scheme is used on mycroft as well.
Attachment #198019 - Flags: review?(mconnor) → review+
Attachment #198019 - Attachment description: Blow away old searchplugins, rev. 1 → Blow away old searchplugins, rev. 1 [checked in on trunk]
Attachment #198019 - Flags: approval1.8b5? → approval1.8b5+
Attachment #198019 - Attachment description: Blow away old searchplugins, rev. 1 [checked in on trunk] → Blow away old searchplugins, rev. 1 [checked in on trunk and 1.8 branch]
Ok, some explanation on how we resolve this bug in total: This bug is blocking 1.5, but not 1.5beta2, as we decided to blow away the existing search engine plugins in the installer. This comes with several benefits for most of our users: - Starting with 1.5, newly added search plugins will be added to the profile, not the installation directory, so for those few that had custom search engine plugins installed, they found the right time to update to the new scheme. - Our release notes always recommended blowing away the previous install completely, so following our recommended install path, it doesn't change anything. - Even with the rename of search engine plugins, we won't end up with doubled entries. This is profile migration for a new feature, to some extent, and yes, it involves user action, but that's the way it is. We will add a note to the release notes for this. Localizers, please adjust your search engine plugins for this. And please cvs remove search engine plugins that are not in list.txt, too. Chris, Rafael, Benjamin, we should probably use unique names for the search engine plugins, too, right? Like Google.com, Google.de etc?
Forgot to add, the URL scheme https://addons.mozilla.org/searchplugins/updates/plugin[-ab-CD].src got signed off by Mike Morgan and Chris Beard and is the one to be used.
Chris, Mike, these are the URLs. Are you OK with the update interval? It's daily for google, every 3 days for the others. I'd like to get that signed off both from the trademarks and the server side.
Comment on attachment 198217 [details] [diff] [review] move the update URL over to https://addons why is the google interval shorter? I don't think its going to be updated any more often than the others. Other than that, looks ok.
A further note to localizers, there is no problem with having 404s on addons unless we actually want to update a particular search engine plugin.
(In reply to comment #23) > no problem with having 404s Except hitting www.mozilla.org every time they use a search plugin instead of every 3 days.
(In reply to comment #24) > Except hitting www.mozilla.org every time they use a search plugin > instead of every 3 days. Could you give us an lxr link for that? I'd really like to know which code is responsible for that.
(In reply to comment #25) > Could you give us an lxr link for that? I'd really like to know which code is > responsible for that. http://lxr.mozilla.org/mozilla/source/xpfe/components/search/src/nsInternetSearchService.cpp#5202 >5202 if (httpStatus != 200) return(NS_ERROR_UNEXPECTED); That line means we are going to exit from OnStopRequest(...) *before* calling validateEngineNow(). Then localstore.rdf will never remember Last-Update-Check-Date as long as the server returns 404. When localstore.rdf doesn't know that date, updateCheckDays=3 is of no use.
Is this a known bug? Because that seems like fairly broken behaviour to me. Can we just call validateEngineNow() on a 404? I don't think that'd be a problem, and the patch would be trivial.
(In reply to comment #27) > Is this a known bug? Because that seems like fairly broken behaviour to me. > Can we just call validateEngineNow() on a 404? I don't think that'd be a > problem, and the patch would be trivial. No, it's not filed yet. It was a bit difficult to find this issue until bug 290254 fixed, but this behavior itself exists since a long long time ago. And it's resonable in a sense, as we failed that check anyway. I'm not too sure we should write down Last-Check-Date even on an invalid update check. Though I agree this behavior sucks in total. And yes, the patch should be trivial.
Regarding rev. 1 patch, you guys realize that software update does not know how to remove directories, right? You're okay with the searchplugins directory being removed at install time only, right?
Yes, at the current point in time updates within the 1.5 timeframe are not a big problem.
If each localized build would have completely localized search plugins, we don't need searchconfig.properties, witch adds extra options to the querry URLs, any longer? http://lxr.mozilla.org/mozilla/source/other-licenses/branding/firefox/content/searchconfig.properties http://lxr.mozilla.org/mozilla/source/browser/base/branding/searchconfig.properties As for what "client=firefox" is doing, please take a look at bug 310075.
Attachment #198217 - Flags: review?(morgamic) → review+
Comment 31 is unrelated, the stuff in other-licenses is merely to ease the trademarks review process.
Due to me being at EuroOSCON (and offline there), this is really late. But we need this fix to do trademarks reviews.
Attachment #198217 - Flags: superreview?(cbeard) → superreview+
landed on the branch. I hope we're OK with the daily update of the google plugin.
This is a trivial patch which I'd like to get into RC1 still. We should really use case sensitive filenames for update. Zilch risk.
Comment on attachment 200721 [details] [diff] [review] eBay.src should update from eBay.src instead of ebay.src oy, good catch
Attachment #200721 - Flags: review?(rebron) → review+
Attachment #200721 - Flags: approval1.8rc1? → approval1.8rc1+
checked in attachement 200721 on the 1.8 branch.
Did attachment 198217 [details] [diff] [review] and attachment 200721 [details] [diff] [review] land on the trunk? Comment 34 and comment 37 don't mention it, nor do the attachment names. If so, can this bug be resolved?
attachement 198217 and 200721 merged together for check in on the trunk.
Attachment #201357 - Flags: review?(rebron)
landed on the trunk
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs review rebron]
(In reply to comment #29) > Regarding rev. 1 patch, you guys realize that software update does not know how > to remove directories, right? You're okay with the searchplugins directory > being removed at install time only, right? The good news is, at least when doing an incremental update, the patching routine removes the old plugins and adds the new ones. So folks updating from b2 to rc1 won't end up with duplicate entries :-), other folks will likely start with an installer build of 1.5 and be good this way around.
*** Bug 318196 has been marked as a duplicate of this bug. ***
(In reply to comment #42) > *** Bug 318196 has been marked as a duplicate of this bug. *** > The old user installed plugins should be moved to their user profile instead of being deleted altogether with the new upgrade.
You need to log in before you can comment on or make changes to this bug.