Closed
Bug 335605
Opened 19 years ago
Closed 19 years ago
Hook up "Download Dictionaries" context menu
Categories
(Firefox :: General, defect, P1)
Tracking
()
RESOLVED
FIXED
Firefox 2 beta2
People
(Reporter: brettw, Assigned: benjamin)
References
Details
(Keywords: fixed1.8.1, Whiteboard: [mustfix][has patch][needs approval])
Attachments
(2 files)
2.77 KB,
patch
|
bugs
:
review+
bugs
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
3.33 KB,
patch
|
brettw
:
review+
mtschrep
:
approval1.8.1+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•19 years ago
|
||
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 2•19 years ago
|
||
Comment on attachment 220812 [details] [diff] [review]
Stopgap patch
This will tide us over through a2.
r+a=ben@mozilla.org
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+
Reporter | ||
Comment 3•19 years ago
|
||
Stopgap patch is on trunk
Updated•19 years ago
|
Flags: blocking-firefox2?
Comment 4•19 years ago
|
||
http://www.mozilla.org/products/thunderbird/dictionaries.html is being redirected to http://www.mozilla.com/thunderbird/dictionaries.html.
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?
Reporter | ||
Comment 5•19 years ago
|
||
As stated in the previous comments, this is a temporary thing because said web page would not be ready for today's alpha2 deadline.
Reporter | ||
Updated•19 years ago
|
Priority: -- → P1
Target Milestone: --- → Firefox 2 beta1
Reporter | ||
Updated•19 years ago
|
Blocks: SpellCheckTracking
Updated•19 years ago
|
Flags: blocking-firefox2? → blocking-firefox2+
Reporter | ||
Updated•19 years ago
|
Whiteboard: swag: 0.5d
Reporter | ||
Updated•19 years ago
|
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
Updated•19 years ago
|
Whiteboard: swag: 0.5d → swag: 0.5d 181b1+
Comment 7•19 years ago
|
||
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 .
Comment 8•19 years ago
|
||
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 http://downloads.mozdev.org/dictionaries/spell-en-GB.xpi . 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.
WORK-ROUND.
Copy/extract dictionary files XX-XX.aiff and XX-XX.dic to <Program Directory>/dictionaries
Comment 10•19 years ago
|
||
Need to get bug 343076 up and running ASAP, will try to poke there.
Whiteboard: swag: 0.5d 181b1+ → [mustfix]
Assignee | ||
Updated•19 years ago
|
Assignee: brettw → benjamin
Assignee | ||
Comment 11•19 years ago
|
||
Attachment #230765 -
Flags: review?
Assignee | ||
Updated•19 years ago
|
Attachment #230765 -
Flags: review? → review?(brettw)
Assignee | ||
Updated•19 years ago
|
Whiteboard: [mustfix] → [mustfix][patch r?brettw]
Reporter | ||
Comment 12•19 years ago
|
||
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+
Reporter | ||
Updated•19 years ago
|
Whiteboard: [mustfix][patch r?brettw] → [mustfix]
Assignee | ||
Comment 13•19 years ago
|
||
I don't think so... we don't want to hardcode addons.mozilla.org URLs, and nothing else makes sense.
Assignee | ||
Comment 14•19 years ago
|
||
Fixed on trunk. The AMO entry points don't actually work yet, but shaver promises they will.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 15•19 years ago
|
||
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?
Updated•19 years ago
|
Whiteboard: [mustfix] → [mustfix][has patch][needs approval]
Updated•19 years ago
|
Attachment #230765 -
Flags: approval1.8.1? → approval1.8.1+
Comment 17•19 years ago
|
||
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:
https://addons.mozilla.org/fr%2C%20fr-fr%2C%20en-us%2C%20en/firefox/2.0b1/dictionaries/
Assignee | ||
Comment 18•19 years ago
|
||
Sylvain, what's wrong with that? If the user wants English content, we give them English content.
Comment 19•19 years ago
|
||
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".
Assignee | ||
Comment 20•19 years ago
|
||
I still don't understand how that breaks things. We probably won't have a fr-ch l10n of addons.mozilla.org, so we'll fall back to whatever's available.
Comment 21•19 years ago
|
||
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:
https://addons.mozilla.org/%LOCALE%/firefox/%VERSION%/dictionaries/
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:
https://addons.mozilla.org/fr%2C%20fr-fr%2C%20en-us%2C%20en/firefox/2.0b1/dictionaries/
https://addons.mozilla.org/fr-ch%2C%20fr%2C%20fr-fr%2C%20en-us%2C%20en/firefox/2.0b1/dictionaries/
https://addons.mozilla.org/fr/firefox/2.0b1/dictionaries/
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 ;-)
Comment 22•19 years ago
|
||
Shaver wrote (bug 343076 comment #17):
> The URL will be
>
> http://addons.mozilla.org/$locale/$appname/$appversion/dictionaries/
>
> 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.
Description
•