Language attributes with country codes not recognized when building link toolbar
Categories
(SeaMonkey :: UI Design, defect)
Tracking
(seamonkey2.53+ fixed, seamonkey2.57esr? affected)
People
(Reporter: dan, Assigned: jobbautista9)
References
Details
(Whiteboard: SM2.53.8)
Attachments
(3 files, 3 obsolete files)
1.01 KB,
text/html
|
Details | |
2.46 KB,
patch
|
iannbugzilla
:
review+
frg
:
feedback+
iannbugzilla
:
approval-comm-release+
iannbugzilla
:
approval-comm-esr60+
|
Details | Diff | Splinter Review |
19.20 KB,
image/png
|
Details |
When an author uses a link tag with a language attribute, e.g., to indicate translated versions of a document, they seem to only be properly recognized in the link toolbar when the simple two-letter language code is used, not when a country code is appended. For instance: <LINK rel="Alternate" href="dummy-en.html" title="In English" lang="en" hreflang="en"> <LINK rel="Alternate" href="dummy-es.html" title="En español" lang="es" hreflang="es"> is properly recognized, as shown by the link bar giving the values as "English: In English" and "Spanish: En español" respectively (which, incidentally, can be redundant if the language is mentioned in the title). However, if country codes are used: <LINK rel="Alternate" href="dummy-en.html" title="In English" lang="en-US" hreflang="en-US"> <LINK rel="Alternate" href="dummy-es.html" title="En español" lang="es-MX" hreflang="es-MX"> then the link menu shows a colon before the title, like it's separating a null language value from the title. This seems to mean that it doesn't recognize the language at all if it has a country code. Ideally, it should either properly parse and recognize the country code, producing values like "American English" (or "English (US)") and "Mexican Spanish: (or "Spanish (Mex.)"), or else strip out the country code and recognize the base language as English or Spanish. Instead, it seems to regard the attribute as an indivisible whole and not recognize it at all.
Updated•22 years ago
|
Testcase which shows this bug. You need to enable the link toolbar, use: View -> Show/Hide -> Site Navigation Bar -> Show Always (or) Show Only As Needed.
Comment 2•20 years ago
|
||
Reassigning obsolete bugs to their respective Seamonkey owners (i.e. nobody). If you want this fixed for Firefox, change the Product and Component accordingly and reassign back to me.
Updated•20 years ago
|
Comment 3•16 years ago
|
||
Filter "spam" on "guifeatures-nobody-20080610".
Assignee | ||
Comment 4•3 years ago
|
||
This looks like something that should be resolved by adding the missing language-country codes in chrome://global/locale/languageNames.properties.
Assignee | ||
Comment 5•3 years ago
|
||
Or never mind, that's actually a bad idea.
I think this still can be solved though. We can test if hreflang contains a dash, and if it does, we get the last two characters, convert it into lowercase, and search in chrome://global/locale/regionNames.properties. Let me see if I can do this.
Assignee | ||
Comment 6•3 years ago
|
||
Ok, I got it fixed, here's the code:
LinkElementDecorator.prototype.makeLongTitle =
function()
{
var prefix = "";
var dash = new RegExp("-");
// XXX: lookup more meaningful and localized version of media,
// i.e. media="print" becomes "Printable" or some such
// XXX: use localized version of ":" separator
if (this.media && !/\ball\b|\bscreen\b/i.test(this.media))
prefix += this.media + ": ";
if (this.hreflang) {
try {
if (!gLanguageBundle)
gLanguageBundle = document.getElementById("languageBundle");
if (!gRegionBundle)
gRegionBundle = document.getElementById("regionBundle");
if (dash.test(this.hreflang)) { // Test if hreflang contains country code
prefix = prefix + gLanguageBundle.getString(this.hreflang.substring(0, this.hreflang.indexOf("-"))) + " (" + gRegionBundle.getString(this.hreflang.substring(this.hreflang.indexOf("-") + 1).toLowerCase()) + ")";
} else {
prefix += gLanguageBundle.getString(this.hreflang);
}
}
catch (e) {
prefix += "???";
}
prefix += ": ";
}
return this.title ? prefix + this.title : prefix;
}
I'm new to this, so feedback would be appreciated!
Assignee | ||
Comment 7•3 years ago
|
||
I also defined gRegionBundle as a variable (i.e. var gRegionBundle;
) first almost a hundred lines back, forgot to mention that in the previous comment.
Comment 8•3 years ago
•
|
||
Job just stumbled over this by accident viewing recently changed bugs. If you can do a patch against the current source in either gitlab or comm-central you can ask me or IanN for review or feedback.
Assignee | ||
Comment 9•3 years ago
|
||
Did I do it right? This is the first time I created a patch.
Assignee | ||
Updated•3 years ago
|
Comment 10•3 years ago
|
||
As a drive by comment:
using split("-") might create a neater solution
I did look at bug 2800 but I couldn't see what should happen when it is an unrecognised region / language. Is this specified somewhere?
Assignee | ||
Comment 11•3 years ago
|
||
(In reply to Ian Neal from comment #10)
As a drive by comment:
using split("-") might create a neater solution
Yeah, good idea.
I'm working on a better solution btw, the undefineds from getString is just giving me trouble. I think I can figure this out though.
I did look at bug 2800 but I couldn't see what should happen when it is an unrecognised region / language. Is this specified somewhere?
I have no idea. I will just use "???" as a placeholder for now. It might even stay since it doesn't need to be localized.
Assignee | ||
Comment 12•3 years ago
|
||
Ok, patch this to the patch.
Assignee | ||
Comment 13•3 years ago
|
||
Here's a better testcase that includes unrecognized hreflang values.
Comment 14•3 years ago
|
||
(In reply to Job Bautista from comment #12)
Created attachment 9219737 [details] [diff] [review]
1255-Bug-134436-Simplify-fix-better-variable-declarations.patchOk, patch this to the patch.
Looks lots neater, thanks.
I would merge the two patches into one as it makes it easier to review. Also when you add a new patch you can obsolete the old patch(es) and request a review / feedback.
As you are moving global variables to only being used within the function change them to languageBundle and regionBundle.
As regards the format of comments, where possible they should start with a capital letter and finish with a fulls stop, and rather than putting comments after the ; have them on a line of their own before the line of code e.g.
instead of:
let ISOcode = this.hreflang.split("-"); // In case hreflang contains country code
have:
// In case hreflang contains country code.
let ISOcode = this.hreflang.split("-");
Is it a region code rather than a country code?
Assignee | ||
Comment 15•3 years ago
|
||
Ok, thanks for the feedback. Please review.
Assignee | ||
Updated•3 years ago
|
Comment 16•3 years ago
|
||
Comment on attachment 9219743 [details] [diff] [review]
Final patch of Bug 134436
Adding review request so I don't forget
Comment 17•3 years ago
•
|
||
Comment on attachment 9219743 [details] [diff] [review]
Final patch of Bug 134436
Job Patch is in git format but imports fine so all good. We are still using hg internally using a big patch queue with the old comm-release repo.
Seems to do the job but I would recommend using a new language variable instead of ??? so that localizers can translate it. Not sure if ? is appropriate for all languages.
Patch added added to our unofficial repo for testing. Available in the next 2.53.8b1 pre build from https://www.wg9s.com/comm-253/
Comment 18•3 years ago
|
||
Comment 19•3 years ago
|
||
Comment on attachment 9219743 [details] [diff] [review]
Final patch of Bug 134436
[Triage Comment]
Thanks r/a=me
Comment 20•3 years ago
|
||
(In reply to Frank-Rainer Grahl (:frg) from comment #17)
Comment on attachment 9219743 [details] [diff] [review]
Final patch of Bug 134436Job Patch is in git format but imports fine so all good. We are still using hg internally using a big patch queue with the old comm-release repo.
Seems to do the job but I would recommend using a new language variable instead of ??? so that localizers can translate it. Not sure if ? is appropriate for all languages.Patch added added to our unofficial repo for testing. Available in the next 2.53.8b1 pre build from https://www.wg9s.com/comm-253/
Created follow-up bug 1709443 for this.
Comment 21•3 years ago
|
||
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/d167bcda83b0
Recognize language attributes with region codes. r=IanN
Updated•3 years ago
|
Comment 22•3 years ago
|
||
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/d73c59b31a39644d2dbccb07971c7b5b1ba99f5e
Recognize language attributes with region codes. r=IanN a=IanN
Assignee | ||
Updated•3 years ago
|
Description
•