Closed
Bug 306576
Opened 19 years ago
Closed 19 years ago
Search plugins URLs should be standardized on *secure* mozilla.org servers
Categories
(Firefox :: Search, defect, P1)
Firefox
Search
Tracking
()
RESOLVED
FIXED
People
(Reporter: benjamin, Assigned: Pike)
References
Details
(Keywords: fixed1.8, late-l10n)
Attachments
(4 files)
(deleted),
patch
|
mconnor
:
review+
asa
:
approval1.8b5+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
morgamic
:
review+
asa
:
superreview+
asa
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mconnor
:
review+
mtschrep
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
rebron
:
review+
|
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".
Reporter | ||
Updated•19 years ago
|
Priority: -- → P1
Comment 1•19 years ago
|
||
When this bug is fixed, bug 284456 will disappear
without any client-side hack.
Comment 2•19 years ago
|
||
(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).
Comment 3•19 years ago
|
||
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.
Comment 4•19 years ago
|
||
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.
Comment 5•19 years ago
|
||
"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.
Comment 6•19 years ago
|
||
The idea sounds good, but I would rather use "google.de" and not "google-de". It
looks a lot more familiar for users.
Comment 7•19 years ago
|
||
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.
Comment 8•19 years ago
|
||
(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.
Comment 9•19 years ago
|
||
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.
Comment 10•19 years ago
|
||
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).
Assignee | ||
Comment 11•19 years ago
|
||
Requesting beta 2 blocking, this has quite some fallout for l10n.
Flags: blocking1.8b5?
Reporter | ||
Comment 12•19 years ago
|
||
Pike, I'm on vacation next week, are you interested in taking/driving this?
Updated•19 years ago
|
Flags: blocking1.8b5? → blocking1.8b5+
Assignee | ||
Comment 14•19 years ago
|
||
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.
Comment 15•19 years ago
|
||
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.
Comment 16•19 years ago
|
||
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.
Assignee | ||
Comment 17•19 years ago
|
||
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.
Updated•19 years ago
|
Flags: blocking1.8b5+ → blocking1.8b5-
Reporter | ||
Comment 18•19 years ago
|
||
Attachment #198019 -
Flags: review?(mconnor)
Updated•19 years ago
|
Attachment #198019 -
Flags: review?(mconnor) → review+
Reporter | ||
Updated•19 years ago
|
Attachment #198019 -
Flags: approval1.8b5?
Reporter | ||
Updated•19 years ago
|
Attachment #198019 -
Attachment description: Blow away old searchplugins, rev. 1 → Blow away old searchplugins, rev. 1 [checked in on trunk]
Updated•19 years ago
|
Attachment #198019 -
Flags: approval1.8b5? → approval1.8b5+
Reporter | ||
Updated•19 years ago
|
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]
Assignee | ||
Comment 19•19 years ago
|
||
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?
Assignee | ||
Comment 20•19 years ago
|
||
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.
Assignee | ||
Comment 21•19 years ago
|
||
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.
Attachment #198217 -
Flags: superreview?(cbeard)
Attachment #198217 -
Flags: review?(morgamic)
Comment 22•19 years ago
|
||
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.
Assignee | ||
Comment 23•19 years ago
|
||
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.
Comment 24•19 years ago
|
||
(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.
Assignee | ||
Comment 25•19 years ago
|
||
(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.
Comment 26•19 years ago
|
||
(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.
Comment 27•19 years ago
|
||
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.
Comment 28•19 years ago
|
||
(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.
Comment 29•19 years ago
|
||
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?
Reporter | ||
Comment 30•19 years ago
|
||
Yes, at the current point in time updates within the 1.5 timeframe are not a big
problem.
Comment 31•19 years ago
|
||
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.
Updated•19 years ago
|
Attachment #198217 -
Flags: review?(morgamic) → review+
Assignee | ||
Comment 32•19 years ago
|
||
Comment 31 is unrelated, the stuff in other-licenses is merely to ease the
trademarks review process.
Assignee | ||
Comment 33•19 years ago
|
||
Due to me being at EuroOSCON (and offline there), this is really late.
But we need this fix to do trademarks reviews.
Flags: blocking1.8rc1?
Updated•19 years ago
|
Attachment #198217 -
Flags: superreview?(cbeard) → superreview+
Updated•19 years ago
|
Attachment #198217 -
Flags: approval1.8rc1+
Assignee | ||
Comment 34•19 years ago
|
||
landed on the branch. I hope we're OK with the daily update of the google plugin.
Updated•19 years ago
|
Flags: blocking1.8rc1? → blocking1.8rc1+
Assignee | ||
Comment 35•19 years ago
|
||
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.
Attachment #200721 -
Flags: review?(rebron)
Attachment #200721 -
Flags: approval1.8rc1?
Comment 36•19 years ago
|
||
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+
Updated•19 years ago
|
Attachment #200721 -
Flags: approval1.8rc1? → approval1.8rc1+
Assignee | ||
Comment 37•19 years ago
|
||
checked in attachement 200721 on the 1.8 branch.
Comment 38•19 years ago
|
||
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?
Assignee | ||
Comment 39•19 years ago
|
||
attachement 198217 and 200721 merged together for check in on the trunk.
Attachment #201357 -
Flags: review?(rebron)
Updated•19 years ago
|
Whiteboard: [needs review rebron]
Updated•19 years ago
|
Attachment #201357 -
Flags: review?(rebron) → review+
Assignee | ||
Comment 40•19 years ago
|
||
landed on the trunk
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [needs review rebron]
Assignee | ||
Comment 41•19 years ago
|
||
(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.
Comment 42•19 years ago
|
||
*** Bug 318196 has been marked as a duplicate of this bug. ***
Comment 43•19 years ago
|
||
(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.
Description
•