Closed Bug 335605 Opened 18 years ago Closed 18 years ago

Hook up "Download Dictionaries" context menu


(Firefox :: General, defect, P1)

2.0 Branch



Firefox 2 beta2


(Reporter: brettw, Assigned: benjamin)



(Keywords: fixed1.8.1, Whiteboard: [mustfix][has patch][needs approval])


(2 files)

The context menu for the browser has a dictionaries part when you click on a textbox. The last item is download dictionaries, which isn't currently hooked up to anything.
Attached patch Stopgap patchSplinter Review
This patch displays dictionary download instructions in an alert box. We will remove this and replace it with a nice web page later.
Attachment #220812 - Flags: review?(bugs)
Attachment #220812 - Flags: approval-branch-1.8.1?(bugs)
Comment on attachment 220812 [details] [diff] [review]
Stopgap patch

This will tide us over through a2.
Attachment #220812 - Flags: review?(bugs)
Attachment #220812 - Flags: review+
Attachment #220812 - Flags: approval-branch-1.8.1?(bugs)
Attachment #220812 - Flags: approval-branch-1.8.1+
Stopgap patch is on trunk
Flags: blocking-firefox2? is being redirected to

And having to write down the url instead of being able to click on a link is lame. Why don't we just open a web page with basically the same text as the dialog and a link to the thunderbird dictionary page?
As stated in the previous comments, this is a temporary thing because said web page would not be ready for today's alpha2 deadline.
Priority: -- → P1
Target Milestone: --- → Firefox 2 beta1
Flags: blocking-firefox2? → blocking-firefox2+
Whiteboard: swag: 0.5d
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
Whiteboard: swag: 0.5d → swag: 0.5d 181b1+
Server-side issues being handled in bug 343076
Depends on: 343076
The UI I think we want for Firefox 2 is to have the "Add Dictionaries ..." menuitem to open a new tab with focus aimed at the page being created by bug 343076.

We'll also need some sort of confirmation that a dictionary has been installed when we click on a link. Nothing fancy, just a "%s dictionary files added." dialog .
From an l10n/trademarks perspective, I think we should have the URL in a pref,
but not a localizable pref. I doubt we really want to have local sites for this, now that AMO is reasonably close to getting localized. It may not make it for fx2, but probably for the lifetime of it, which is good enough for me.
The stopgap patch suggested (and implemented) in comment 2 does not work (at least for the . The install.js script in that xpi file places the dictionary files in <Program Directory>/components/myspell but firefox 2.0b1 expects to find them in <Program Directory>/dictionaries. 


Copy/extract dictionary files XX-XX.aiff and XX-XX.dic to <Program Directory>/dictionaries
Need to get bug 343076 up and running ASAP, will try to poke there.
Whiteboard: swag: 0.5d 181b1+ → [mustfix]
Assignee: brettw → benjamin
Attachment #230765 - Flags: review? → review?(brettw)
Whiteboard: [mustfix] → [mustfix][patch r?brettw]
Comment on attachment 230765 [details] [diff] [review]
Open a new tab/window to get new dictionaries from addons, rev. 1

Looks OK to me. Should we have some hardcoded fallback if the pref isn't found?
Attachment #230765 - Flags: review?(brettw) → review+
Whiteboard: [mustfix][patch r?brettw] → [mustfix]
I don't think so... we don't want to hardcode URLs, and nothing else makes sense.
Fixed on trunk. The AMO entry points don't actually work yet, but shaver promises they will.
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 230765 [details] [diff] [review]
Open a new tab/window to get new dictionaries from addons, rev. 1

Need to verify in hourly/nightlies.
Attachment #230765 - Flags: approval1.8.1?
Whiteboard: [mustfix] → [mustfix][has patch][needs approval]
Attachment #230765 - Flags: approval1.8.1? → approval1.8.1+
Keywords: fixed1.8.1
Shouldn't "general.useragent.locale" be used instead of "intl.accept_languages"?

If someone changes the intl.accept_languages setting through the ui (for instance, moving english on top), then the URL won't be valid any more.

For french, it tries to access:
Sylvain, what's wrong with that? If the user wants English content, we give them English content.
The problem I see is the following:

A user downloads Firefox with French localization. By default the accept language is "fr, fr-fr, en-us, en". If that user comes from Switzerland she may want to add the French from Switzerland (fr-ch) localization using the preference panel. After  that, the link to the dictionaries will be broken, as the intl parameter will be "fr-ch,fr,fr-fr,en-us,en".
I still don't understand how that breaks things. We probably won't have a fr-ch l10n of, so we'll fall back to whatever's available.
This may not be an issue depending on how the server side component deals with the URL's that are used to access the dictionaries.

In fact, when I first saw the URL which is used to access those:

I thought that the server side implementation would have a set of directories for all possible "intl.accept_languages" values that are set by default for each Firefox localized versions.

So I thought: what if a user changed the intl.accept_languages configuration variable using the preferences? Maybe the user would face a 404 Error page instead of getting to the correct dictionary for the current locale.

For instance, all the following URL's:

Should redirect to the French dictionary page

With a bit of URL rewriting and some server side processing logic, that is not an issue. I was just raising this because I thought at first it would be more easy for the server side implementation to parse the general.useragent.locale value (as there would be exactly one possibility for each Firefox locale, and that parameter is not modifiable through the ui by default).

Please forget my noise if I am wrong here ;-)

Shaver wrote (bug 343076 comment #17):
> The URL will be
> from which we will likely redirect to another part of the site.  $locale should
> refer to the locale in which we want content (_("These are extra dictionaries
> you can add to enhance Firefox's spell-checking capabilities"), etc.) to be
> presented.

This rather reads like $locale should be exactly one locale.

Although this keeps me wondering why we need to indicate a locale at all, since that one should rather be read from the Accept-Languages header anyway.
You need to log in before you can comment on or make changes to this bug.