Closed Bug 136556 Opened 23 years ago Closed 13 years ago

Image title should use "×", not "x".

Categories

(Core :: Internationalization: Localization, defect)

defect
Not set
trivial

Tracking

()

VERIFIED FIXED
mozilla9

People

(Reporter: neil, Assigned: aceman)

References

()

Details

(Keywords: intl, polish)

Attachments

(2 files, 4 obsolete files)

Using Build ID: 2002040903 Steps to reproduce problem: 1. Right-click on the mozilla banner and select "View Image" Expected Result: mozilla-banner.gif (GIF image, 600×58 pixels) Actual Result: mozilla-banner.gif (GIF image, 600x58 pixels)
Attached patch Proposed Patch (obsolete) (deleted) — Splinter Review
Keywords: patch, polish, review
Attached patch use \ud7 instead of × (obsolete) (deleted) — Splinter Review
I'm not sure which charset .properties files are in... I'd use \ud7 to be on the safe side, as done here.
Attachment #78523 - Attachment is obsolete: true
Attached patch \u00d7 perhaps? (obsolete) (deleted) — Splinter Review
Whoa. Has this been tested on all the platforms (we have chronic problems with non-ASCII characters in the titlebar). Has this been tested on systems on which the "default" encoding is not a western encoding but rather UTF8 or UCS-16 or Shift-JIS or something like that? I'll give this a shot on Linux with a typically dumb windowmanager tonight...
Keywords: intl
neil, uh, your patch contains two diffs for the same file... the first one being your original patch, the last one being correct bz, I have tested this on Linux/Sawfish
Yeah... I'm not sure whether sawfish is non-ASCII-challenged... This still needs testing in non-western locales XP.
OK, testing on afterstep (still the standard "C" locale) shows that even in braindead windowmanagers this works correctly. :) Any chance of getting an intl person to test this on a non-Western setup?
OS: Windows 95 → All
Hardware: PC → All
I don't know how can I apply the patch in my build. cc Roy.
i'll try it on my shift-jis build. cc'ing tao
this breaks me on for apps: shelf and pwm using xphoton running on QNX6.0a (mozilla is xlib freebsd) I don't think there is any benefit in doing this. Interestingly enough, the first time i loaded this page in this qnx voyager, the title appeared as: Image title should use '' , not "x". And the description contained: Expected Result: mozilla-banner.gif (GIF image, 600 8 pixels) Actual Result: mozilla-banner.gif (GIF image, 600x58 pixels) Now in qnx_voyager, I see the symbol you want me to see. But mozilla currently shows 60058 as the dimension (both in window title and shelf)
useless correction gtwm is the x windowmanager, pwm is the photon windowmanager.
> neil, uh, your patch contains two diffs for the same file... the first one being > your original patch, the last one being correct sorry, caused by a >> vs > typo :-)
Component: ImageLib → Image: Layout
Component: Image: Layout → XP Apps: GUI Features
QA Contact: tpreston → sairuh
i'm sending this over to l10n in hopes that they can make some kind of call on this. if not, send it to xpapps.
Assignee: pavlov → rchen
Component: XP Apps: GUI Features → Localization
QA Contact: sairuh → ruixu
QA Contact: ruixu → ylong
give me a break. Don't spend time on this kind of issue. BTW, I think it should be fine for ANY environment with the path since the characters is in ISO-8859-1 and any platform could display it as long as it can display western european languages. However, I don't think this patch is worth of any attention to review. reassign this bug back to the reporter.
Assignee: rchen → neil
Comment on attachment 78558 [details] [diff] [review] \u00d7 perhaps? r=bzbarsky If Frank says this should be fine, then I'm all for it. It looks much better.
Attachment #78558 - Flags: review+
Unfortunately, the Mac sucks. Is there any chance we can do this in a platform specific way?
Well, we could rewrite the unicode to MacRoman conversion tables so that \u00d7 maps to 0x78 "x". Interestingly if you try to paste \u00d7 into an MS-DOS window Windows will translate it into an x for you, although Java says it shouldn't.
And on solaris as well, according to Pike on #mozilla. NSObject says OS X works.
Depends on: 141707
Depends on: 36689
No longer depends on: 141707
On plaforms with a properly i18nized 'window manager' (Win2k/XP, *nix with XFree86 and WMs implementing _NET_WM spec/extended ICCCM spec, and MacOS X), any Unicode characters can be put in the window title bar regardless of the present locale at least in principle. On other platforms (e.g. MacOS 9 or earlier, *nix with WMs not supporting extended ICCCM spec, and Win 9x/ME), whether a given Unicode character is displayable in the title bar depends on the character repertoire of the present locale (or more exactly the locale under which 'window manager' is launched). As Frank wrote, I don't think it's wise to put this in now. U+00D7 is better than lowercase x, but is it better to such an extent to warrant a brekage in old platforms (and perhaps some I18N-challenged embeded devices)? I wouldn't say the answer is yes.
FYI, the character shows properly for me in the initial description and summary field here, but in the summary in the bug search results, it shows as the generic replacement character (diamond with question mark). SuSE 10.1 with self-compiled Firefox. (And, FWIW, I also wonder why bother.)
QA Contact: amyy → localization
This bug seems stalled. I think today firefox no longer supports those "other platforms" (MacOS 9 or earlier, *nix with WMs not supporting extended ICCCM spec, and Win 9x/ME). I understand this may not be worth bothering, but today it may not be much of a bother. Just apply the change without worries. Or is there any problem?
(In reply to aceman from comment #21) > *** Bug 677912 has been marked as a duplicate of this bug. *** Not entirely a duplicate. Non-breaking spaces, please. Thanks.
We already use unicode characters in other places in the UI. I don't expect any problem with modern platforms. Who wants to clean up the patch?
(Note that these strings are now in dom/locales/en-US/chrome/layout/MediaDocument.properties)
Attached patch updated patch (obsolete) (deleted) — Splinter Review
Patch updated to MediaDocument.properties and also incorporates bug 677912.
Attachment #78553 - Attachment is obsolete: true
Attachment #78558 - Attachment is obsolete: true
Attachment #556118 - Flags: review?(bzbarsky)
Comment on attachment 556118 [details] [diff] [review] updated patch r=me
Attachment #556118 - Flags: review?(bzbarsky) → review+
Attachment #556118 - Flags: ui-review?(dao)
Attachment #556118 - Flags: superreview?(neil)
This patch does not do non-breaking spaces as bug 677912 envisioned -- around the times symbol, not linking it with the word 'pixels'. The correct string should be |%S\u00a0\u00d7\u00a0%S pixels|
You are right, will fix that.
Attached patch fixed new patch (deleted) — Splinter Review
Attachment #556118 - Attachment is obsolete: true
Attachment #556158 - Flags: ui-review?(dao)
Attachment #556158 - Flags: superreview?(neil)
Attachment #556158 - Flags: review?(bzbarsky)
Attachment #556118 - Flags: ui-review?(dao)
Attachment #556118 - Flags: superreview?(neil)
Comment on attachment 556158 [details] [diff] [review] fixed new patch r=me
Attachment #556158 - Flags: review?(bzbarsky) → review+
Comment on attachment 556158 [details] [diff] [review] fixed new patch this doesn't need super- or ui-review
Attachment #556158 - Flags: ui-review?(dao)
Attachment #556158 - Flags: superreview?(neil)
Ok, then somebody please merge it. Thanks.
Assignee: neil → acelists
Pushed changeset d813440eb90b to mozilla-central.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
[removing checkin-needed]
Keywords: checkin-needed
Is causing Moth orange on m-c: { TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/content/html/document/test/browser_bug592641.js | Title should be correct on load #1 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/content/html/document/test/browser_bug592641.js | Title should be correct on load #2 } http://tbpl.allizom.org/?usebuildbot=1&rev=d813440eb90b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Looks like that test just needs updated to account for the new title format. Should be trivial to fix. /content/html/document/test/browser_bug592641.js 17 function checkTitle(title) { 18 19 ctx.loadsDone++; 20 ok(/^bug592641_img\.jpg \(JPEG Image, 1500x1500 pixels\)/.test(title), 21 "Title should be correct on load #" + ctx.loadsDone); 22 }
Thanks, I will look into that.
Attachment #556943 - Flags: review?(dolske)
Attachment #556943 - Flags: review?(dolske) → review+
Pushed http://hg.mozilla.org/integration/fx-team/rev/19db4fd8a771 Will make its way into mozilla-central next time someone does a merge (probably a day or three). Thanks for the patch!
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla9
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Verified with Mozilla/5.0 (X11; Linux i686; rv:9.0a1) Gecko/20110911 Firefox/9.0a1 ID:20110911030845
Status: RESOLVED → VERIFIED
Blocks: 677912
Aceman, I am suprised to see today the bug in Firefox 23.0.1, on Mac OS X. The letter “x” and the lack of spaces. Both in the window title and in the info window. You may see these screen photos : http://pbrd.co/16Sxr7h http://pbrd.co/16SELzV
This is fixed in the default English strings, but not French: http://hg.mozilla.org/l10n-central/fr/file/71f17b1c476c/dom/chrome/layout/MediaDocument.properties You'll probably want to open a new bug for fixing the French locale.
Nicolas, is that in English Firefox? Aaron, yes, this was one of my first patches and this time nobody noticed I need to change the string ID so that translators notice the change. So it is quite possible some localizations still hold onto the old string. I can fix that, just tell me if I should add the new patch here or somebody files a new bug. I still think bug 677912 is a duplicate of this.
Flags: needinfo?(bzbarsky)
Actually I can attach the fix to bug 677912 so we do not open a new one just for this.
(In reply to Aaron Kaluszka from comment #46) > This is fixed in the default English strings, but not French: > http://hg.mozilla.org/l10n-central/fr/file/71f17b1c476c/dom/chrome/layout/ > MediaDocument.properties You'll probably want to open a new bug for fixing > the French locale. No, I'll not want. :-) This is not a different bug, I will not create a different bug file for every language. (In reply to :aceman from comment #47) > Nicolas, is that in English Firefox? Aceman, my Firefox is in french. Thank you for your action.
Please file a new bug for any followup work, if that's what you were asking for needinfo on. If not, what did you want to know? ;)
Flags: needinfo?(bzbarsky)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: