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

VERIFIED FIXED in mozilla9

Status

()

Core
Localization
--
trivial
VERIFIED FIXED
16 years ago
4 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: aceman)

Tracking

({intl, polish})

Trunk
mozilla9
intl, polish
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

16 years ago
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)
(Reporter)

Comment 1

16 years ago
Created attachment 78523 [details] [diff] [review]
Proposed Patch
(Reporter)

Updated

16 years ago
Keywords: patch, polish, review
Created attachment 78553 [details] [diff] [review]
use \ud7 instead of ×

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
(Reporter)

Comment 3

16 years ago
Created attachment 78558 [details] [diff] [review]
\u00d7 perhaps?

Comment 4

16 years ago
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

Comment 6

16 years ago
Yeah... I'm not sure whether sawfish is non-ASCII-challenged...  This still
needs testing in non-western locales XP.

Comment 7

16 years ago
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

Comment 8

16 years ago
I don't know how can I apply the patch in my build.  cc Roy.

Comment 9

16 years ago
i'll try it on my shift-jis build.
cc'ing tao 

Comment 10

16 years ago
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)

Comment 11

16 years ago
useless correction gtwm is the x windowmanager, pwm is the photon 
windowmanager.
(Reporter)

Comment 12

16 years ago
> 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 :-)

Updated

16 years ago
Component: ImageLib → Image: Layout

Updated

16 years ago
Component: Image: Layout → XP Apps: GUI Features
QA Contact: tpreston → sairuh

Comment 13

16 years ago
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

Updated

16 years ago
QA Contact: ruixu → ylong

Comment 14

15 years ago
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 15

15 years ago
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?
(Reporter)

Comment 17

15 years ago
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.
(Reporter)

Comment 18

15 years ago
And on solaris as well, according to Pike on #mozilla. NSObject says OS X works.
(Reporter)

Updated

15 years ago
Depends on: 141707

Updated

15 years ago
Depends on: 36689
No longer depends on: 141707

Comment 19

15 years ago
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. 

Comment 20

11 years ago
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
(Assignee)

Updated

6 years ago
Duplicate of this bug: 677912
(Assignee)

Comment 22

6 years ago
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?

Comment 23

6 years ago
(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)
Duplicate of this bug: 677912
(Assignee)

Comment 27

6 years ago
Created attachment 556118 [details] [diff] [review]
updated patch

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 28

6 years ago
Comment on attachment 556118 [details] [diff] [review]
updated patch

r=me
Attachment #556118 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

6 years ago
Attachment #556118 - Flags: ui-review?(dao)
Attachment #556118 - Flags: superreview?(neil)

Comment 29

6 years ago
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|
(Assignee)

Comment 30

6 years ago
You are right, will fix that.
(Assignee)

Comment 31

6 years ago
Created attachment 556158 [details] [diff] [review]
fixed new patch
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 32

6 years ago
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)
(Assignee)

Comment 34

6 years ago
Ok, then somebody please merge it. Thanks.

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Assignee: neil → acelists
(Reporter)

Comment 35

6 years ago
Pushed changeset d813440eb90b to mozilla-central.
Status: NEW → RESOLVED
Last Resolved: 6 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
Backed out: 
http://hg.mozilla.org/mozilla-central/rev/85d9c87ef45a
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 }
(Assignee)

Comment 40

6 years ago
Thanks, I will look into that.
(Assignee)

Comment 41

6 years ago
Created attachment 556943 [details] [diff] [review]
patch for test browser_bug592641.js
(Assignee)

Updated

6 years ago
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
http://hg.mozilla.org/mozilla-central/rev/19db4fd8a771
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
(Assignee)

Comment 44

6 years ago
Verified with Mozilla/5.0 (X11; Linux i686; rv:9.0a1) Gecko/20110911 Firefox/9.0a1 ID:20110911030845
Status: RESOLVED → VERIFIED

Updated

4 years ago
Blocks: 677912

Comment 45

4 years ago
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

Comment 46

4 years ago
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.
(Assignee)

Comment 47

4 years ago
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)
(Assignee)

Comment 48

4 years ago
Actually I can attach the fix to bug 677912 so we do not open a new one just for this.

Comment 49

4 years ago
(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.

Comment 50

4 years ago
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.