Status

defect
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: kairo, Assigned: kairo)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

We should move the searchplugins we currently have in xpfe/components/search/datasets to suite/locales so localizers can include their own possibly modified list.
This patch does practically the same as Firefox does, just using the old sherlock plugins instead of the new XML search stuff Firefox has (SeaMonkey doesn't support that yet). Using Axel for review as he knows the Makefile logic we're using here.
Additionally to this patch, all of mozilla/xpfe/components/search/datasets can be cvs removed and the .src and .gif files from there need to be added in the new place at mozilla/suite/locales/en-US/searchplugins/
Attachment #271425 - Flags: superreview?(neil)
Attachment #271425 - Flags: review?
Attachment #271425 - Flags: review? → review?(l10n)
Comment on attachment 271425 [details] [diff] [review]
move searchplugins to locale

>+SEARCH_PLUGINS = $(shell cat $(LOCALE_SRCDIR)/searchplugins/list.txt)
I think you want this to be := so you only shell out once.

>\ Kein Zeilenumbruch am Dateiende.
Unnecessary, surely?
Attachment #271425 - Flags: superreview?(neil) → superreview+
(In reply to comment #2)
> (From update of attachment 271425 [details] [diff] [review])
> >+SEARCH_PLUGINS = $(shell cat $(LOCALE_SRCDIR)/searchplugins/list.txt)
> I think you want this to be := so you only shell out once.

Done locally.

> >\ Kein Zeilenumbruch am Dateiende.
> Unnecessary, surely?

I guess so - this wasn't intended. Also added the linebreak locally.
Comment on attachment 271425 [details] [diff] [review]
move searchplugins to locale

Don't you allow png icons for search?

As a nit, I'd group the vpaths by file type, not by location, I guess that would read better, and flows along "get src from en-US and l10n, get gif from en-US and l10n..."
(In reply to comment #4)
> (From update of attachment 271425 [details] [diff] [review])
> Don't you allow png icons for search?

I don't know if our search engine stuff detects PNGs there, I always have only seen GIFs there.

> As a nit, I'd group the vpaths by file type, not by location, I guess that
> would read better, and flows along "get src from en-US and l10n, get gif from
> en-US and l10n..."

Sure, that's no problem :)
I recall at least png files from back in the days when Firefox used sherlock.

As per IRC, 
<KaiRo>	Pike: hmm, I see the search code supprts .gif/.jpg/.jpeg/.png

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/locales/Makefile.in&rev=1.39&mark=129&cvsroot=/cvsroot#128 says that we only supported .gif and .png in CVS, though.
Hmm, I'm not sure how to make this work with still including the fallback solution to ship the en-US plugin of the same name if it's not present in the L10n, as this solution like currently in the patch requires the named files to be present, and a "gif or png" approach makes this harder.

I think it's no big problem if we just support one image format for that solution - we can make this be PNG instead of GIF if you like that better, though. I don't think it matters a lot what format it is though, as those are 16x16 files (and I think GIF is dominant there).
Blocks: 286110
Comment on attachment 271425 [details] [diff] [review]
move searchplugins to locale

I don't mind whether you use gif or png.

As I'm not going to be the one that needs to answer the questions on why just one, I'm fine with the patch.

I guess one could $(wildcard) in both l10n and en-US and then find out if you need to do something about multiple icons or such, but that's not really nice nor vital.
Attachment #271425 - Flags: review?(l10n) → review+
From what I've seen on L10n CVS for old Firefox versions that still used sherlock plugins, most of the non-google  plugins localizers added were PNG (we have Google in the default set, so they can use our icon - GIF and PNG wqere pretty equal in total when Google was included).

Because of that, I converted to PNG for SeaMonkey on checkin. I think it's reasonable to use a single image format, as conversion is easy.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.