Following bug 373623, also in the places where ellipses are introduced programmatically, to shorten long strings, the unicode symbol should be used.

So, you should probably use the new "intl.ellipsis" pref here instead of hardcoding the ellipsis.

Yes, this sounds like a case for intl.ellipsis - esp. as there's actually something shortended there. And all such characters should go through some place in localization anyways, as in some languages (like Japanese), the unicode ellipsis is not rendered in the way we want it and needs to be replaced with dots (which, of course, can nicely be done through intl.ellipsis).

Comment on attachment 292213 [details] [diff] [review] introduces … in nsContextMenu.js per previous comments

(In reply to comment #5) > Created an attachment (id=298444) [details] > Patch using intl.ellipsis rv1.0 Looks good to me, in as far as I can tell with my limited knowledge of JS. You’ll want to ask someone to review this, Gavin, I suppose. I am sorry not to have tackled this myself. I had a try, but it didn’t work, and then christmas holidays and computer trouble hindered me from getting to it.

Comment on attachment 298444 [details] [diff] [review] Patch using intl.ellipsis rv1.0 >Index: browser/base/content/nsContextMenu.js I'd just use: >+ this.ellipsis = "\u2026"; // default value >+ try { >+ this.ellipsis = gPrefService.getComplexValue("intl.ellipsis", >+ Ci.nsIPrefLocalizedString).data; >+ } catch (e) { } and put it directly in the nsContextMenu constructor. The rest looks fine. I'll r+ a new patch with that change made.

Comment on attachment 298659 [details] [diff] [review] Patch using intl.ellipsis rv2.0 Use the correct ellipsis in code, too.

(In reply to comment #9) > (From update of attachment 298659 [details] [diff] [review]) > Use the correct ellipsis in code, too. > Is it needed to replace "\u2026" as "…" ?

(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 298659 [details] [diff] [review] [details]) > > Use the correct ellipsis in code, too. > Is it needed to replace "\u2026" as "…" ? Ah, that comment was addressed to drivers, not to you. Don't worry about that comment. :)

Sorry my patch contains CR-LF, please replace it as LF.

(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 298659 [details] [diff] [review] [details]) > > Use the correct ellipsis in code, too. > > > Is it needed to replace "\u2026" as "…" ? Maybe not, but I don’t see why not?

Gavin, is that change ok?

The current patch is fine.

Comment on attachment 298659 [details] [diff] [review] Patch using intl.ellipsis rv2.0 a=beltzner for 1.9 ... :)

Checking in browser/base/content/nsContextMenu.js; /cvsroot/mozilla/browser/base/content/nsContextMenu.js,v <-- nsContextMenu.js new revision: 1.32; previous revision: 1.31 done

Verified at Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012404 Minefield/3.0b3pre