Closed Bug 373623 Opened 18 years ago Closed 17 years ago

Unicode … should be used instead of three separate dots: ...

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: hendrik, Assigned: hendrik)

References

Details

Attachments

(10 files, 13 obsolete files)

(deleted), application/vnd.mozilla.xul+xml
Details
(deleted), patch
sipaq
: review+
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
hendrik
: review+
Details | Diff | Splinter Review
(deleted), patch
mconnor
: approval1.9+
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), patch
Pike
: review-
Details | Diff | Splinter Review
(deleted), patch
philor
: review+
Details | Diff | Splinter Review
Attached patch Substitutes all triple dots (obsolete) (deleted) — Splinter Review
The GUI often contains ‘...’ In most places, this is entered in the source as three dots. However, there is a nicer-looking unicode symbol for this, which should be used wherever possible. Its code is \U2026. I am unsure how this should be percolated to l10ns. The attached patch takes care of this (it also removes a superfluous newline somewhere).
If this gets accepted, I can deliver similar patches for mail, editor, toolkit etc.
Aaron, is there an a11y-impact of ... vs … ?
There could very likely be, because the text-to-speech engines that screen readers use may not be able to handle this. We'd have to do a lot of testing and communication with screen readers to find out. If this is important, go ahead and check it in to trunk and we'll see what happens.
Also, I forgot to mention Braille displays. Screen readers control Braille displays like these: http://www.deafblind.com/display.html The Braille translation engine probably does not know that unicode symbol.
The unicode '…' symbol is already widely used in Sunbird (though not always consequently), e.g. http://lxr.mozilla.org/mozilla1.8/source/calendar/locales/en-US/chrome/calendar/menuOverlay.dtd
Sunbird hasn't been tested for accessibility yet. This is going to take a fair amount of testing. I'm not trying to kill your patch, just saying how it is.
Do other popular, accessible apps on Windows and Gnome already use this symbol? If so then it's probably safe.
Aaron, is this testcase enough to check what happens in either? I only used buttons, not menus, not sure if the difference would matter.
(In reply to comment #4) > The Braille translation engine probably does not know that unicode symbol. Then the developers of those engines have some work to do. I say this is not our problem. Seeing the symbol being used in Sunbird is indeed the reason for this patch.
Can I get an answer to my question in comment 7? If it's already used all over the place, then it's obviously not an issue. If it's not used, then we're just being different because we can. I'm not sure why we'd want our ... to look different from what other apps have. Hendrik, welcome to the world of accessibility. Like I said I'm not trying to kill your patch but Axel asked a great question. Is it a problem? Very possibly. We don't have time to test the zillion screen readers, text-to-speech and Braille display engines out there to create the proper gap analysis :/ Hence comment 7.
How is this different than bug 365624?
Also note bug 255051 - ellipsis should /not/ be used everywhere ! (but then there should be no dot-dot-dot either)
Aaron, see bug 365624, which I agree this is probably a dupe of. The Apple HIG says that the ellipsis character is better for accessibility than the three dots. http://developer.apple.com/documentation/UserExperience/Conceptual/OSXHIGuidelines/XHIGText/chapter_13_section_3.html#//apple_ref/doc/uid/TP30000365-DontLinkElementID_218
Okay, that's on Apple. How about Windows and Linux?
(In reply to comment #12) > Also note bug 255051 - ellipsis should /not/ be used everywhere ! (but then > there should be no dot-dot-dot either) bug 255051 is about whether or not to use ellipsis on some places. This bug is about using the proper symbol *if* ellipsis are used. Thus they are only very loosely related.
Assignee: nobody → hendrik.maryns
OS: Linux → All
Hardware: PC → All
I saw you assigned it to me, that’s fine, but note that my contribution to this bug is finished now. I have no cvs commit permissions.
Status: NEW → ASSIGNED
Not having write access is no barrier to having the bug assigned - along with begging your eventual reviewer for checkin, or asking on IRC, you can always add [checkin needed] to the Whiteboard field. But before that you need review per http://www.mozilla.org/projects/firefox/review.html and before you can ask for review, you or someone interested in seeing this land needs to come up with a reasonably authoritative answer to comment 7 - do other popular accessible apps on Windows and Linux use ellipses rather than three dots?
(In reply to comment #8) > Created an attachment (id=258292) [details] > XUL with two buttons, the first with ... the second with hellip > Aaron, is this testcase enough to check what happens in either? I only used > buttons, not menus, not sure if the difference would matter. I tested this in the two most popular screen-readers. In both cases, they recognized the ellipses symbol for what it was, using speech. I'm reasonably certain that I remember the same applying to braille, but I'll need to do some checking to coroborate this. However, in the menu overlay for Sunbird, here's what came across for one of the menu entries that I presume is supposed to have an ellipses. <!ENTITY calendar.open.file.label "Open Calendar File…"> What's with the junk at the end of that? My screen-readers tell me "A circumflex, euro, vertical bar". If that's supposed to be the ellipses symbol, I have some additional testing to do to reconcile the differences between these two cases.
Interesting. On an intel mac with 10.4.9, VoiceOver reports all of these menu options as ending with "ellipsis". version: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.3pre) Gecko/20070315 Calendar/0.5pre
Tim, when you say "they recognized the ellipses symbol for what it was", does that mean they speak something different from the "...". It won't be confusing for screen reader users will it?
Having tested in Sunbird, the menu entries with ellipses don't read the same as menu entries with dot-dot-dot. The AT products do recognize the ellipses, however, they just don't automatically read it if you're arrowing through a menu. Bottom line: this patch could be applied without negatively affecting the ability of Jaws/Window-eyes/screen magnifier users' to use the application. A minor tweak on the AT developer side should allow users to get consistent feedback regardless of whether a tripple dot or an ellipses is used. Braille translation tables include the ellipses symbol, so there's no reason it should negatively affect them either. I recall from my Apple System 7 days that Mac applications always used the ellipses, or at least most of the time, so it doesn't surprise me that Voiceover reads them correctly. On the Linux side, even if the AT products don't recognize the ellipses, it won't cause users to be unable or less able to use the application.
(In reply to comment #21) > Tim, when you say "they recognized the ellipses symbol for what it was", does > that mean they speak something different from the "...". It won't be confusing > for screen reader users will it? It won't be confusing for a screen-reader user. The menu will read the same with the same voice inflection, but it won't read the dot-dot-dot at the end. Different users have different punctuation settings, and even between Firefox and Internet Explorer, I get different menu behavior. So, if it's important to interproduct look and feel, this patch could be applied without negatively affecting screen-reader users. If the only purpose is because the ellipses symbol looks better, I would think the "if it ain't broke..." rule applies.
Here's my takeaway, which is slightly different I think: - It is different but not something users can't figure out, but it does say something sensible. - We'll have to tell AT vendors to change their code, if we want it to be the same, which they probably won't because they have bigger issues. - It's not enough of a reason for users to upgrade to a newer AT - Many Windows screen reader users will still have the older AT no matter what, and hear something different in our app than they normally hear. Since it's different some small number of users who are less tech-savvy might be confused initially, but will get used to it. - This is in fact different from what other Windows apps are doing. All and all it's not a huge deal either way, but might annoy some. Then again since it's not really a big deal we probably shouldn't hold up the patch.
My last comment was written before I read your comment 23. Anwyay, I'm a little confused now. Ellipses don't get spoken or do? If they're not spoken, why won't screen reader users be confused? Don't they need to know the ... is there so they know when something will open a dialog? It's the same queue sighted users get.
Which screen reader are you asking about? If VoiceOver on a Mac (10.4.9, intel) is given "Open File..." (with three dots), it says "Open File, ellipsis". If it is given "Open File..." (with ellipses), it says "Open File, ellipsis".
(In reply to comment #25) > My last comment was written before I read your comment 23. > > Anyway, I'm a little confused now. Ellipses don't get spoken or do? > > If they're not spoken, why won't screen reader users be confused? Don't they > need to know the ... is there so they know when something will open a dialog? > It's the same queue sighted users get. Yes, but most blind users have textual punctuation turned off, so they don't hear the dot-dot-dot, they only rely on the inflection of the speech. With ellipses, this inflection is the same. Some will not hear the dot-dot-dot, and might be confused initially. So, My thinking is that while it might raise an eyebrow, the user will still know what that particular menu function or button will do. There are many situations where buttons have the three dots on the end of them, but this is not read by screen-readers because of the default punctuation setting. In these cases the ellipses would make no difference, as users would hear the same voice inflection. Regardless, if no other applications use this symbol, I don't understand why we would want to defy convention in that way.
Ray, VoiceOver users are not yet common, as most blind users are still on Windows. Historically, that's where the most complete access has been, by far. So generally we're talking about Window-Eyes and JAWS. Hal/Supernova is also used on Windows, a bit less commonly, and it doesn't work with Firefox. A newer option is NVDA, an open source screen reader for Windows.
(In reply to comment #19) > However, in the menu overlay for Sunbird, here's > what came across for one of the menu entries that I presume is supposed to have > an ellipses. > <!ENTITY calendar.open.file.label "Open Calendar File…"> > What's with the junk at the end of that? My screen-readers tell me "A > circumflex, euro, vertical bar". That's UTF-8 being treated as Windows-1252 - lxr doesn't specify an encoding, so if you don't have autodetect on (View - Character Encoding - Autodetect - Universal) you'll get it in your OS's default encoding. Not a problem when it's being used, only when it's being displayed as a web page. (In reply to comment #25) > If they're not spoken, why won't screen reader users be confused? Don't they > need to know the ... is there so they know when something will open a dialog? Which is where the relationship to bug 255051 that Hendrik is sure doesn't exist comes in: ellipses do not mean "this will open a dialog" but rather "you will have to provide more information before the action this item's label suggests will actually happen." Since both users and developers are uniformly confused about the correct meaning, it's not that strong of a cue - even for the exact same action going through the exact same UI, ellipsis-or-not varies from one application to another.
(In reply to comment #27) > Regardless, if no other applications use this symbol, I don't understand why > we would want to defy convention in that way. Since technology evolves. In the old days, three dots was the way to express an ellipsis, because there was no other option, or because one had no access to the correct glyph. Now, in the days of Unicode, we can do better.
Depends on: 317702
Depends on: 390282
Time to ask for review? I filed bug 390282 for XUL labels.
from bug 390282 comment 1: > This really needs review from someone who knows how common this character is in > fonts I suppose that issue applies to this bug as well. Any idea who could help out?
Depends on: 390333
(In reply to comment #32) > from bug 390282 comment 1: > > This really needs review from someone who knows how common this character is in > > fonts > > I suppose that issue applies to this bug as well. ... resolved as per bug 390333. Bug 390282 is now ready for checkin. Once that's done, this one should get high priority for UI consistency.
Attached patch updated patch (obsolete) (deleted) — Splinter Review
An updated version of the patch, now asking for review.
Attachment #258281 - Attachment is obsolete: true
Attachment #280098 - Flags: ui-review?
Attachment #280098 - Flags: review?
I tried mconnor for review, but bugzilla didn’t like that. What should I do? What about patches for calendar, TB, toolkit etc., should I make separate bugs for them?
I guess you can attach one big patch and ask Axel Hecht (l10n@mozilla.com) for review. If more reviews for the various modules are needed, these can be added to that patch.
Attached patch patch for calendar (obsolete) (deleted) — Splinter Review
most of calendar already uses correct ellipsis
Attachment #280110 - Flags: review?
Attachment #280098 - Flags: review? → review?(l10n)
Attachment #280110 - Flags: review? → review?(l10n)
Attached patch patch for editor (obsolete) (deleted) — Splinter Review
Attachment #280112 - Flags: review?
Attachment #280112 - Flags: review? → review?(l10n)
Comment on attachment 280110 [details] [diff] [review] patch for calendar Properties files are utf-8 encoded, and you're actually reverting the right char in chrome/calendar/calendar.properties. Real utf-8 is easier to read, let's stick with that instead of \U2026.
Attachment #280110 - Flags: review?(l10n) → review-
Attached patch patch for mail (obsolete) (deleted) — Splinter Review
Also removes some obsolete comments and unnecessary whitespace, I can take that out if you wish.
Attachment #280114 - Flags: review?(l10n)
Attached patch patch for security (obsolete) (deleted) — Splinter Review
Attachment #280116 - Flags: review?(l10n)
Comment on attachment 280098 [details] [diff] [review] updated patch r- on ... in utf-8 vs \U2026 in .properties, too.
Attachment #280098 - Flags: review?(l10n) → review-
Too many patches alert. Please submit one for browser/toolkit, one for mail/editor, and one for calendar, with the review comments addressed.
Attached patch patch for toolkit (obsolete) (deleted) — Splinter Review
Sorry for the bugspam, this is the last one.
Attachment #280117 - Flags: review?(l10n)
Attached patch patch for browser/toolkit and security, tested (obsolete) (deleted) — Splinter Review
I managed building it, the menus look fine, it also works in help windows and preferences.
Attachment #280098 - Attachment is obsolete: true
Attachment #280116 - Attachment is obsolete: true
Attachment #280117 - Attachment is obsolete: true
Attachment #280200 - Flags: review?(l10n)
Attachment #280116 - Flags: review?(l10n)
Attachment #280117 - Flags: review?(l10n)
Attachment #280098 - Flags: ui-review?
(In reply to comment #45) For what it worth, we've been using real ellipsis for several months in fr locale for all products and we didn't find any bugs about it.
Attached patch mail & editor, tested (obsolete) (deleted) — Splinter Review
Attachment #280112 - Attachment is obsolete: true
Attachment #280114 - Attachment is obsolete: true
Attachment #280205 - Flags: review?(l10n)
Attachment #280112 - Flags: review?(l10n)
Attachment #280114 - Flags: review?(l10n)
Attached patch calendar (deleted) — Splinter Review
Attachment #280110 - Attachment is obsolete: true
Attachment #280210 - Flags: review?(l10n)
Comment on attachment 280200 [details] [diff] [review] patch for browser/toolkit and security, tested r=me, with the following nits: browser/locales/en-US/installer/installer.inc toolkit/locales/en-US/installer/unix/install.it toolkit/locales/en-US/installer/windows/install.it are not used anymore. browser/locales/en-US/installer/*.properties is in utf-8 in the source, but the build converts them to cp1252 for use in nsis. Please check that locally, though I think I was able to check it locally. More or a rs on the rest of the changes.
Attachment #280200 - Flags: review?(l10n) → review+
I forgot, updater.ini is the same as installer properties.
Comment on attachment 280205 [details] [diff] [review] mail & editor, tested rs=me with the same nits on installer files and updater as for browser.
Attachment #280205 - Flags: review?(l10n) → review+
Comment on attachment 280210 [details] [diff] [review] calendar Calendar isn't really my business. Same nits as previous apply. Note, my reviews here are all rubber stamps, you'll need to get the actual tree-policy relevant reviews on top of my comments here.
Attachment #280210 - Flags: review?(l10n) → review?
Comment on attachment 280210 [details] [diff] [review] calendar duh, I can't clear me as reviewer without requesting review myself, and I don't want that. Clearing the review flag completely instead.
Attachment #280210 - Flags: review?
Attachment #280210 - Flags: review?(bugzilla)
Attachment #280205 - Flags: review?(mscott)
Attachment #280200 - Flags: review?(mconnor)
(In reply to comment #49) > browser/locales/en-US/installer/*.properties is in utf-8 in the source, but > the build converts them to cp1252 for use in nsis. Please check that locally, > though I think I was able to check it locally. I did iconv -f utf-8 -t cp1252 *.properties in all three installer folders. It gave no error message. The ellipsis was not visible on my console, but it was if I saved the result in a temp file and opened it with cp1252 encoding, so I think this is fine.
Comment on attachment 280205 [details] [diff] [review] mail & editor, tested I think this is a dupe of another bug that Simon posted a patch for too. But anyway, this looks good to me.
Attachment #280205 - Flags: review?(mscott) → review+
Comment on attachment 280210 [details] [diff] [review] calendar Looks good. r=sipaq Please contact me before checking this in, as this will have to be committed to the Mozilla 1.8 branch as well and only after we lift our current string freeze.
Attachment #280210 - Flags: review?(bugzilla) → review+
For all three patches, i.e. browser mail calendar
Keywords: checkin-needed
You still need review and approval for browser and toolkit.
And I don't think calendar has lifted their string freeze yet. Looks like your "mail & editor" patch is actually just mail, which is probably a good thing, since editor has different owners/peers than mail. You probably want to unobsolete your editor patch, and get Neil to r+sr it. But the mail patch, which seems to be the only thing that's actually ready for checkin-needed, has gotten enough bit-rot that it no longer applies - you'll probably find it easier to get someone to check it in if you attach an unrotted version.
Keywords: checkin-needed
Don't forget that this bug is trying to fix a moving target. Since the last patch, there have been other changes in the GUI that also need a change to \U2026 : bug 400237. Also note that bug 400237 comment 3 complains that \U2026 is too wide in the Japanese MS UI Gothic font.
Flags: blocking-firefox3?
Wanted, but not a blocker ...
Flags: blocking-firefox3? → blocking-firefox3-
Whiteboard: [wanted-firefox3]
Attachment #280200 - Flags: review?(mconnor) → review+
Attachment #280200 - Flags: approval1.9?
Attached patch patch for browser locales, tested (obsolete) (deleted) — Splinter Review
Ok, I decided to do this properly now. 1. I checked out a fresh wc yesterday. (with make client.mk co and project=browser) 2. I replaced all ... by … in all .dtd and .properties files, without looking at them. 3. I went through all matches of ... in all files (about 10 000) and evaluated whether they should be replaced. I tried - not to replace them in comments - to replace them in print statements etc. Some have definitely escaped my scrutiny, but I am rather confident I got it right. The test build survived. 4. Noticing that there is some code which inserts ... in certain places, I edited that code as well, as far as I was confident enough with what it was doing. 5. I split up the diff in 5 patches: - locale: all the stuff which is in a locales/ directory. These should be unproblematic. Dots in comments are probably replaced here as well. - code: the parts where I changed stuff other than just dots; please review this a bit more thoroughly - locale-other: for some strange reason, the inspector contains locales for other languages than en-US. I put these in a separate patch since I am unsure what has to happen with them. -misc: here is all the stuff from other directories. These are mainly c and js files with print or debug statements. Also configure files, readmes, docs and htmls (bug descriptions, mostly). For some of those files, I had to convert them to UTF-8, but I think that is an Eclipse issue. It doesn’t show up in the patch. If this is a problem, let me know what I can do about it. You probably want to consider whether you want to check-in the misc stuff, since a lot of it is command-line interaction, which uses fixed-width fonts, and … in fixed-width looks ugly. But of course I’d prefer it to be done! I hope to find some time in the near future to go through the same routine for TB and SM. The above was two weeks ago. Just in time I thought about building and testing first. It segfaulted during startup. Probably since I did some file encoding conversions. So I made a fresh checkout again and am now testing the patches. This is the one for locales, which probably does not need another review, but I’ll ask it anyway. I guess it should not be contested. The others will follow soon.
Attachment #280200 - Attachment is obsolete: true
Attachment #287896 - Flags: review?
Attachment #280200 - Flags: approval1.9?
Attachment #287896 - Flags: review? → review?(mconnor)
Attached patch patch for other locales in inspector (obsolete) (deleted) — Splinter Review
I put this in a separate patch, since it does not only affect en-US. I have no idea why these are downloaded along with en-US in a normal checkout?? Tested and fine
Attachment #287907 - Flags: review?(mconnor)
Attached patch patch for code inserting ellipsis (obsolete) (deleted) — Splinter Review
I found out that in several places, there is code which cuts off strings and appends ellipsis at the end. I edited this code. This patch contains all those instances. Unfortunately the sqlite3-files also contain other stuff, which you might not want. This is related to the upcoming patch: it replaces the ellipsis in all other locations, that is, outside of l10n-strings, in debug-output etc. But certainly, some of this should get checked in. I leave it to the reviewers to decide how much of it (maybe wait for the next attachment).
Attachment #288197 - Flags: review?(mconnor)
No longer depends on: 403484
Comment on attachment 288197 [details] [diff] [review] patch for code inserting ellipsis Please file (at least) one new bug for this, and please don't patch sqlite3 and prototype. You also need to ask the right people for review, see <http://www.mozilla.org/owners.html>.
Attachment #288197 - Attachment is obsolete: true
Attachment #288197 - Flags: review?(mconnor)
Comment on attachment 287907 [details] [diff] [review] patch for other locales in inspector Locales other than en-US are maintained by dedicated teams. Please don't touch them.
Attachment #287907 - Attachment is obsolete: true
Attachment #287907 - Flags: review?(mconnor)
Comment on attachment 287896 [details] [diff] [review] patch for browser locales, tested Why are you changing comments? Why are you patching xpfe?
(In reply to comment #66) > (From update of attachment 288197 [details] [diff] [review]) > Please file (at least) one new bug for this, You mean for this code patch? Ok, will do. With ‘at least’, you probably mean I should split up this patch in several components, each with its own reviewer? Could you maybe point out which parts are meaningful together? > and please don't patch sqlite3 and > prototype. Why not? > You also need to ask the right people for review, see > <http://www.mozilla.org/owners.html>. I’ll try! (In reply to comment #68) > (From update of attachment 287896 [details] [diff] [review]) > Why are you changing comments? I tried not to, but really, in such a long list of changes, one overlooks little things. Is it that bad? > Why are you patching xpfe? Because I can? I checked out the code needed for browser and changed all occurrences of three dots that are inside strings in ellipsis. There is one more patch coming up, which does all the strings which are not in l10n files. But since you are sceptical here already (which I expected, btw): does it make sense? Should I file yet another bug for that? It changes strings in debug output, logs, .txt and html files, … I am trying to build with this now. (In reply to comment #67) > (From update of attachment 287907 [details] [diff] [review]) > Locales other than en-US are maintained by dedicated teams. Please don't touch > them. Makes sense, but why are these checked out for an English build? Other bug, I guess, should I file it? Can somebody help me a bit getting this structured? That is: which bugs should I file, which patches where and who to ask for review.
(In reply to comment #69) > Could you maybe point out which parts are meaningful together? See "Find Module" on http://www.mozilla.org/owners.html > > and please don't patch sqlite3 and > > prototype. > > Why not? Because that will make it harder to keep them up to date, I guess. I also don't see why they should be changed in the first place. > > Why are you changing comments? > > I tried not to, but really, in such a long list of changes, one overlooks > little things. Is it that bad? It's not particularly bad, it's just completely unneeded. > > Why are you patching xpfe? > > Because I can? I checked out the code needed for browser and changed all > occurrences of three dots that are inside strings in ellipsis. Hrm, I don't think xpfe/global/resources/locale/en-US/charsetOverlay.dtd is built with Firefox, for example. > There is one > more patch coming up, which does all the strings which are not in l10n files. > But since you are sceptical here already (which I expected, btw): does it make > sense? Should I file yet another bug for that? It changes strings in debug > output, logs, .txt and html files, No, I don't think changing debug output makes sense. As for txt and html files, it depends on whether they're part of the UI. > Makes sense, but why are these checked out for an English build? Other bug, I > guess, should I file it? Dunno.
Attached patch misc patch (deleted) — Splinter Review
Just so you understand what I am talking about: the patch for ‘all other files’. It looks nice to see my ellipsises (?) flash by if I run my configure file… Damn, too large, well, just an arbitrary first part of it then.
(In reply to comment #70) > > > Why are you patching xpfe? > > > > Because I can? I checked out the code needed for browser and changed all > > occurrences of three dots that are inside strings in ellipsis. > > Hrm, I don't think xpfe/global/resources/locale/en-US/charsetOverlay.dtd is > built with Firefox, for example. Firefox isn't the only product Mozilla has in its CVS repository. Just because it isn't part of Firefox doesn't mean it shouldn't be fixed. Mozilla isn't completely Firefox-centric. > > Makes sense, but why are these checked out for an English build? Other bug, I > > guess, should I file it? > > Dunno. If Axel is ok with it, it shouldn't be a problem to fix the inspector locales that are in the main tree.
(In reply to comment #72) > (In reply to comment #70) > > > > Why are you patching xpfe? > > Firefox isn't the only product Mozilla has in its CVS repository. Just because > it isn't part of Firefox doesn't mean it shouldn't be fixed. Mozilla isn't > completely Firefox-centric. This is a Firefox bug to start with. Who says that the SeaMonkey guys want that change? Robert Kaiser certainly didn't want it. Also, whoever patches xpfe should patch all files, not a few random ones.
(In reply to comment #73) > (In reply to comment #72) > > (In reply to comment #70) > > > > > Why are you patching xpfe? > > > > Firefox isn't the only product Mozilla has in its CVS repository. Just because > > it isn't part of Firefox doesn't mean it shouldn't be fixed. Mozilla isn't > > completely Firefox-centric. > > This is a Firefox bug to start with. Who says that the SeaMonkey guys want that > change? Robert Kaiser certainly didn't want it. Also, whoever patches xpfe > should patch all files, not a few random ones. True. However, from IRC, it seems that Robert has accepted the inevitable and is willing to do this for SM as well. Since there already were reviews accepted for TB and SB, I guess in the long run the whole codebase will/should be done. That said, I am open for a better methodology, if somebody /please/ explain it to me. Like I mentioned above, I just checked out browser and changed all file that came with it. I am planning to do the same for mail, calendar and suite, in that order, but first things should be resolved here. And yes, I will make separate bugs. I talked to Axel, the inspector locales should not be patched. However, there are 4 files for en-US in that patch which should be patched here. I guess I’d make a new patch for locales which integrates those? I’ll wait for review and comments first.
Comment on attachment 287896 [details] [diff] [review] patch for browser locales, tested r=mconnor we should get this in ASAP
Attachment #287896 - Flags: review?(mconnor) → review+
Attachment #287896 - Flags: approval1.9?
Comment on attachment 288395 [details] [diff] [review] misc patch >Index: browser/components/safebrowsing/content/report-phishing-overlay.xul >=================================================================== >- label="&reportPhishSiteMenu.title;..." >+ label="&reportPhishSiteMenu.title;…" > accesskey="&reportPhishSiteMenu.accesskey;" > insertbefore="updateSeparator" > observes="reportPhishingBroadcaster" > oncommand="openUILink(safebrowsing.getReportURL('Phish'), event);" > onclick="checkForMiddleClick(this, event);"/> > <menuitem id="menu_HelpPopup_reportPhishingErrortoolmenu" >- label="&safeb.palm.notforgery.label;..." >+ label="&safeb.palm.notforgery.label;…" Those are real bugs, please move those ellipsis to the respective DTD, having them in XUL is plain wrong, as locaizers need to be able to localize them, e.g. in Japanese that character often renders wrong in fonts so they might be better off with using three dots in their L10n - but that's completly up to localizers, so it should be in DTDs. The rest in this "misc patch" is probably stuff that shouldn't change. Comment and test fixes are unnecessary and just require that anyone editing the source file has a unicode editor but don't give us any other win, and the prints, etc. go out to the console and I'm not convinced we should require all users to have unicode console fonts on their various system consoles.
(In reply to comment #69) > > Why are you patching xpfe? > > Because I can? Just leave it alone. xpfe/ strings are completely unused now, except for places where Camino still uses them, and they will abandon them hopefully soon as well, when we'll just remove them. No need to patch stuff that is almost completely unused. BTW, SeaMonkey will want this as well - while it's right that I don't particularly like it, being inconsistent between toolkit and SeaMonkey strings is even much worse ;-)
Comment on attachment 287896 [details] [diff] [review] patch for browser locales, tested a=beltzner for drivers
Attachment #287896 - Flags: approval1.9? → approval1.9+
what I'm checking in
Attachment #287896 - Attachment is obsolete: true
attachment 290319 [details] [diff] [review]: Checking in browser/installer/unix/config.it; /cvsroot/mozilla/browser/installer/unix/config.it,v <-- config.it new revision: 1.21; previous revision: 1.20 done Checking in browser/locales/en-US/chrome/browser/baseMenuOverlay.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/baseMenuOverlay.dtd,v <-- baseMenuOverlay.dtd new revision: 1.9; previous revision: 1.8 done Checking in browser/locales/en-US/chrome/browser/browser.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.dtd,v <-- browser.dtd new revision: 1.80; previous revision: 1.79 done Checking in browser/locales/en-US/chrome/browser/browser.properties; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.properties,v <-- browser.properties new revision: 1.53; previous revision: 1.52 done Checking in browser/locales/en-US/chrome/browser/engineManager.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/engineManager.dtd,v <-- engineManager.dtd new revision: 1.6; previous revision: 1.5 done Checking in browser/locales/en-US/chrome/browser/openLocation.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/openLocation.dtd,v <-- openLocation.dtd new revision: 1.4; previous revision: 1.3 done Checking in browser/locales/en-US/chrome/browser/pageInfo.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/pageInfo.dtd,v <-- pageInfo.dtd new revision: 1.23; previous revision: 1.22 done Checking in browser/locales/en-US/chrome/browser/searchbar.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/searchbar.dtd,v <-- searchbar.dtd new revision: 1.3; previous revision: 1.2 done Checking in browser/locales/en-US/chrome/browser/shellservice.properties; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/shellservice.properties,v <-- shellservice.properties new revision: 1.8; previous revision: 1.7 done Checking in browser/locales/en-US/chrome/browser/tabbrowser.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/tabbrowser.dtd,v <-- tabbrowser.dtd new revision: 1.6; previous revision: 1.5 done Checking in browser/locales/en-US/chrome/browser/tabbrowser.properties; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/tabbrowser.properties,v <-- tabbrowser.properties new revision: 1.5; previous revision: 1.4 done Checking in browser/locales/en-US/chrome/browser/feeds/subscribe.properties; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/feeds/subscribe.properties,v <-- subscribe.properties new revision: 1.7; previous revision: 1.6 done Checking in browser/locales/en-US/chrome/browser/history/history.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/history/history.dtd,v <-- history.dtd new revision: 1.5; previous revision: 1.4 done Checking in browser/locales/en-US/chrome/browser/migration/migration.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/migration/migration.dtd,v <-- migration.dtd new revision: 1.7; previous revision: 1.6 done Checking in browser/locales/en-US/chrome/browser/places/places.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/places/places.dtd,v <-- places.dtd new revision: 1.38; previous revision: 1.37 done Checking in browser/locales/en-US/chrome/browser/places/places.properties; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/places/places.properties,v <-- places.properties new revision: 1.32; previous revision: 1.31 done Checking in browser/locales/en-US/chrome/browser/preferences/advanced.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/advanced.dtd,v <-- advanced.dtd new revision: 1.27; previous revision: 1.26 done Checking in browser/locales/en-US/chrome/browser/preferences/content.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/content.dtd,v <-- content.dtd new revision: 1.15; previous revision: 1.14 done Checking in browser/locales/en-US/chrome/browser/preferences/languages.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/languages.dtd,v <-- languages.dtd new revision: 1.6; previous revision: 1.5 done Checking in browser/locales/en-US/chrome/browser/preferences/main.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/main.dtd,v <-- main.dtd new revision: 1.8; previous revision: 1.7 done Checking in browser/locales/en-US/chrome/browser/preferences/preferences.properties; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/preferences.properties,v <-- preferences.properties new revision: 1.21; previous revision: 1.20 done Checking in browser/locales/en-US/chrome/browser/preferences/privacy.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/privacy.dtd,v <-- privacy.dtd new revision: 1.15; previous revision: 1.14 done Checking in browser/locales/en-US/chrome/browser/preferences/security.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/security.dtd,v <-- security.dtd new revision: 1.7; previous revision: 1.6 done Checking in browser/locales/en-US/chrome/help/cookies.xhtml; /cvsroot/mozilla/browser/locales/en-US/chrome/help/cookies.xhtml,v <-- cookies.xhtml new revision: 1.16; previous revision: 1.15 done Checking in browser/locales/en-US/chrome/help/customization.xhtml; /cvsroot/mozilla/browser/locales/en-US/chrome/help/customization.xhtml,v <-- customization.xhtml new revision: 1.26; previous revision: 1.25 done Checking in browser/locales/en-US/chrome/help/firebird-toc.rdf; /cvsroot/mozilla/browser/locales/en-US/chrome/help/firebird-toc.rdf,v <-- firebird-toc.rdf new revision: 1.65; previous revision: 1.64 done Checking in browser/locales/en-US/chrome/help/menu_reference.xhtml; /cvsroot/mozilla/browser/locales/en-US/chrome/help/menu_reference.xhtml,v <-- menu_reference.xhtml new revision: 1.52; previous revision: 1.51 done Checking in browser/locales/en-US/chrome/help/popup.xhtml; /cvsroot/mozilla/browser/locales/en-US/chrome/help/popup.xhtml,v <-- popup.xhtml new revision: 1.17; previous revision: 1.16 done Checking in browser/locales/en-US/chrome/help/prefs.xhtml; /cvsroot/mozilla/browser/locales/en-US/chrome/help/prefs.xhtml,v <-- prefs.xhtml new revision: 1.59; previous revision: 1.58 done Checking in browser/locales/en-US/chrome/help/using_firebird.xhtml; /cvsroot/mozilla/browser/locales/en-US/chrome/help/using_firebird.xhtml,v <-- using_firebird.xhtml new revision: 1.44; previous revision: 1.43 done Checking in browser/locales/en-US/installer/custom.properties; /cvsroot/mozilla/browser/locales/en-US/installer/custom.properties,v <-- custom.properties new revision: 1.13; previous revision: 1.12 done Checking in browser/locales/en-US/installer/installer.inc; /cvsroot/mozilla/browser/locales/en-US/installer/installer.inc,v <-- installer.inc new revision: 1.9; previous revision: 1.8 done Checking in browser/locales/en-US/installer/override.properties; /cvsroot/mozilla/browser/locales/en-US/installer/override.properties,v <-- override.properties new revision: 1.3; previous revision: 1.2 done Checking in browser/locales/en-US/updater/updater.ini; /cvsroot/mozilla/browser/locales/en-US/updater/updater.ini,v <-- updater.ini new revision: 1.6; previous revision: 1.5 done Checking in dom/locales/en-US/chrome/layout/HtmlForm.properties; /cvsroot/mozilla/dom/locales/en-US/chrome/layout/HtmlForm.properties,v <-- HtmlForm.properties new revision: 1.4; previous revision: 1.3 done Checking in dom/locales/en-US/chrome/layout/layout_errors.properties; /cvsroot/mozilla/dom/locales/en-US/chrome/layout/layout_errors.properties,v <-- layout_errors.properties new revision: 1.2; previous revision: 1.1 done Checking in dom/locales/en-US/chrome/layout/xbl.properties; /cvsroot/mozilla/dom/locales/en-US/chrome/layout/xbl.properties,v <-- xbl.properties new revision: 1.7; previous revision: 1.6 done Checking in editor/ui/locales/en-US/chrome/composer/editor.properties; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/composer/editor.properties,v <-- editor.properties new revision: 1.3; previous revision: 1.2 done Checking in editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd,v <-- editorOverlay.dtd new revision: 1.6; previous revision: 1.5 done Checking in editor/ui/locales/en-US/chrome/composer/pref-editing.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/composer/pref-editing.dtd,v <-- pref-editing.dtd new revision: 1.3; previous revision: 1.2 done Checking in editor/ui/locales/en-US/chrome/dialogs/EdDialogOverlay.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/dialogs/EdDialogOverlay.dtd,v <-- EdDialogOverlay.dtd new revision: 1.2; previous revision: 1.1 done Checking in editor/ui/locales/en-US/chrome/dialogs/EditorImageMapHotSpot.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/dialogs/EditorImageMapHotSpot.dtd,v <-- EditorImageMapHotSpot.dtd new revision: 1.2; previous revision: 1.1 done Checking in editor/ui/locales/en-US/chrome/dialogs/EditorImageProperties.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/dialogs/EditorImageProperties.dtd,v <-- EditorImageProperties.dtd new revision: 1.2; previous revision: 1.1 done Checking in editor/ui/locales/en-US/chrome/dialogs/EditorInputProperties.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/dialogs/EditorInputProperties.dtd,v <-- EditorInputProperties.dtd new revision: 1.2; previous revision: 1.1 done Checking in editor/ui/locales/en-US/chrome/dialogs/EditorPublishProgress.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/dialogs/EditorPublishProgress.dtd,v <-- EditorPublishProgress.dtd new revision: 1.3; previous revision: 1.2 done Checking in editor/ui/locales/en-US/chrome/dialogs/EditorSpellCheck.dtd; /cvsroot/mozilla/editor/ui/locales/en-US/chrome/dialogs/EditorSpellCheck.dtd,v <-- EditorSpellCheck.dtd new revision: 1.3; previous revision: 1.2 done Checking in extensions/cck/browser/locales/en-US/chrome/cckWizard.dtd; /cvsroot/mozilla/extensions/cck/browser/locales/en-US/chrome/cckWizard.dtd,v <-- cckWizard.dtd new revision: 1.30; previous revision: 1.29 done Checking in extensions/cck/browser/locales/en-US/chrome/cckwizard.properties; /cvsroot/mozilla/extensions/cck/browser/locales/en-US/chrome/cckwizard.properties,v <-- cckwizard.properties new revision: 1.8; previous revision: 1.7 done Checking in extensions/irc/locales/en-US/chrome/chatzilla.properties; /cvsroot/mozilla/extensions/irc/locales/en-US/chrome/chatzilla.properties,v <-- chatzilla.properties new revision: 1.144; previous revision: 1.143 done Checking in extensions/irc/locales/en-US/chrome/config.dtd; /cvsroot/mozilla/extensions/irc/locales/en-US/chrome/config.dtd,v <-- config.dtd new revision: 1.3; previous revision: 1.2 done Checking in extensions/layout-debug/ui/locale/en-US/layoutdebug.dtd; /cvsroot/mozilla/extensions/layout-debug/ui/locale/en-US/layoutdebug.dtd,v <-- layoutdebug.dtd new revision: 1.6; previous revision: 1.5 done Checking in extensions/reporter/locales/en-US/chrome/reportWizard.dtd; /cvsroot/mozilla/extensions/reporter/locales/en-US/chrome/reportWizard.dtd,v <-- reportWizard.dtd new revision: 1.10; previous revision: 1.9 done Checking in extensions/reporter/locales/en-US/chrome/reportWizard.properties; /cvsroot/mozilla/extensions/reporter/locales/en-US/chrome/reportWizard.properties,v <-- reportWizard.properties new revision: 1.8; previous revision: 1.7 done Checking in extensions/reporter/resources/content/reporter/reporterOverlay.xul; /cvsroot/mozilla/extensions/reporter/resources/content/reporter/reporterOverlay.xul,v <-- reporterOverlay.xul new revision: 1.10; previous revision: 1.9 done Checking in extensions/sql/base/resources/locale/en-US/pref-sql.dtd; /cvsroot/mozilla/extensions/sql/base/resources/locale/en-US/pref-sql.dtd,v <-- pref-sql.dtd new revision: 1.4; previous revision: 1.3 done Checking in extensions/sroaming/resources/locale/en-US/prefs.dtd; /cvsroot/mozilla/extensions/sroaming/resources/locale/en-US/prefs.dtd,v <-- prefs.dtd new revision: 1.4; previous revision: 1.3 done Checking in extensions/sroaming/resources/locale/en-US/transfer.properties; /cvsroot/mozilla/extensions/sroaming/resources/locale/en-US/transfer.properties,v <-- transfer.properties new revision: 1.5; previous revision: 1.4 done Checking in extensions/venkman/resources/locale/en-US/venkman-help.tpl; /cvsroot/mozilla/extensions/venkman/resources/locale/en-US/venkman-help.tpl,v <-- venkman-help.tpl new revision: 1.6; previous revision: 1.5 done Checking in extensions/venkman/resources/locale/en-US/venkman.dtd; /cvsroot/mozilla/extensions/venkman/resources/locale/en-US/venkman.dtd,v <-- venkman.dtd new revision: 1.17; previous revision: 1.16 done Checking in extensions/venkman/resources/locale/en-US/venkman.properties; /cvsroot/mozilla/extensions/venkman/resources/locale/en-US/venkman.properties,v <-- venkman.properties new revision: 1.67; previous revision: 1.66 done Checking in extensions/wallet/editor/resources/locale/en-US/WalletEditor.dtd; /cvsroot/mozilla/extensions/wallet/editor/resources/locale/en-US/WalletEditor.dtd,v <-- WalletEditor.dtd new revision: 1.4; previous revision: 1.3 done Checking in extensions/wallet/resources/locale/en-US/walletTasksOverlay.dtd; /cvsroot/mozilla/extensions/wallet/resources/locale/en-US/walletTasksOverlay.dtd,v <-- walletTasksOverlay.dtd new revision: 1.8; previous revision: 1.7 done Checking in extensions/xforms/resources/locale/en-US/xforms.dtd; /cvsroot/mozilla/extensions/xforms/resources/locale/en-US/xforms.dtd,v <-- xforms.dtd new revision: 1.14; previous revision: 1.13 done Checking in layout/forms/resources/locale/en-US/htmlforms.dtd; /cvsroot/mozilla/layout/forms/resources/locale/en-US/htmlforms.dtd,v <-- htmlforms.dtd new revision: 1.5; previous revision: 1.4 done Checking in netwerk/locales/en-US/necko.properties; /cvsroot/mozilla/netwerk/locales/en-US/necko.properties,v <-- necko.properties new revision: 1.5; previous revision: 1.4 done Checking in security/manager/locales/en-US/chrome/pippki/certManager.dtd; /cvsroot/mozilla/security/manager/locales/en-US/chrome/pippki/certManager.dtd,v <-- certManager.dtd new revision: 1.19; previous revision: 1.18 done Checking in security/manager/locales/en-US/chrome/pippki/deviceManager.dtd; /cvsroot/mozilla/security/manager/locales/en-US/chrome/pippki/deviceManager.dtd,v <-- deviceManager.dtd new revision: 1.3; previous revision: 1.2 done Checking in security/manager/locales/en-US/chrome/pippki/pippki.dtd; /cvsroot/mozilla/security/manager/locales/en-US/chrome/pippki/pippki.dtd,v <-- pippki.dtd new revision: 1.3; previous revision: 1.2 done Checking in security/manager/locales/en-US/chrome/pippki/pippki.properties; /cvsroot/mozilla/security/manager/locales/en-US/chrome/pippki/pippki.properties,v <-- pippki.properties new revision: 1.16; previous revision: 1.15 done Checking in security/manager/locales/en-US/chrome/pippki/pref-masterpass.dtd; /cvsroot/mozilla/security/manager/locales/en-US/chrome/pippki/pref-masterpass.dtd,v <-- pref-masterpass.dtd new revision: 1.5; previous revision: 1.4 done Checking in security/manager/locales/en-US/chrome/pippki/pref-security.dtd; /cvsroot/mozilla/security/manager/locales/en-US/chrome/pippki/pref-security.dtd,v <-- pref-security.dtd new revision: 1.3; previous revision: 1.2 done Checking in security/manager/locales/en-US/chrome/pippki/pref-validation.dtd; /cvsroot/mozilla/security/manager/locales/en-US/chrome/pippki/pref-validation.dtd,v <-- pref-validation.dtd new revision: 1.7; previous revision: 1.6 done Checking in toolkit/locales/en-US/chrome/global/appPicker.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/appPicker.dtd,v <-- appPicker.dtd new revision: 1.2; previous revision: 1.1 done Checking in toolkit/locales/en-US/chrome/global/charsetOverlay.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/charsetOverlay.dtd,v <-- charsetOverlay.dtd new revision: 1.4; previous revision: 1.3 done Checking in toolkit/locales/en-US/chrome/global/keys.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/keys.properties,v <-- keys.properties new revision: 1.3; previous revision: 1.2 done Checking in toolkit/locales/en-US/chrome/global/nsHelperAppDlg.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/nsHelperAppDlg.dtd,v <-- nsHelperAppDlg.dtd new revision: 1.7; previous revision: 1.6 done Checking in toolkit/locales/en-US/chrome/global/nsHelperAppDlg.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/nsHelperAppDlg.properties,v <-- nsHelperAppDlg.properties new revision: 1.5; previous revision: 1.4 done Checking in toolkit/locales/en-US/chrome/global/printPageSetup.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/printPageSetup.dtd,v <-- printPageSetup.dtd new revision: 1.5; previous revision: 1.4 done Checking in toolkit/locales/en-US/chrome/global/printPreview.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/printPreview.dtd,v <-- printPreview.dtd new revision: 1.6; previous revision: 1.5 done Checking in toolkit/locales/en-US/chrome/global/printPreviewProgress.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/printPreviewProgress.dtd,v <-- printPreviewProgress.dtd new revision: 1.4; previous revision: 1.3 done Checking in toolkit/locales/en-US/chrome/global/printProgress.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/printProgress.dtd,v <-- printProgress.dtd new revision: 1.4; previous revision: 1.3 done Checking in toolkit/locales/en-US/chrome/global/printdialog.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/printdialog.dtd,v <-- printdialog.dtd new revision: 1.6; previous revision: 1.5 done Checking in toolkit/locales/en-US/chrome/global/viewSource.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/viewSource.dtd,v <-- viewSource.dtd new revision: 1.6; previous revision: 1.5 done Checking in toolkit/locales/en-US/chrome/global/xpinstall/xpinstall.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/xpinstall/xpinstall.properties,v <-- xpinstall.properties new revision: 1.9; previous revision: 1.8 done Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.dtd,v <-- downloads.dtd new revision: 1.18; previous revision: 1.17 done Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties,v <-- downloads.properties new revision: 1.13; previous revision: 1.12 done Checking in toolkit/locales/en-US/chrome/mozapps/downloads/unknownContentType.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/unknownContentType.dtd,v <-- unknownContentType.dtd new revision: 1.9; previous revision: 1.8 done Checking in toolkit/locales/en-US/chrome/mozapps/downloads/unknownContentType.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/unknownContentType.properties,v <-- unknownContentType.properties new revision: 1.8; previous revision: 1.7 done Checking in toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd,v <-- extensions.dtd new revision: 1.28; previous revision: 1.27 done Checking in toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties,v <-- extensions.properties new revision: 1.45; previous revision: 1.44 done Checking in toolkit/locales/en-US/chrome/mozapps/extensions/update.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/update.dtd,v <-- update.dtd new revision: 1.11; previous revision: 1.10 done Checking in toolkit/locales/en-US/chrome/mozapps/handling/handling.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/handling/handling.dtd,v <-- handling.dtd new revision: 1.3; previous revision: 1.2 done Checking in toolkit/locales/en-US/chrome/mozapps/handling/handling.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/handling/handling.properties,v <-- handling.properties new revision: 1.2; previous revision: 1.1 done Checking in toolkit/locales/en-US/chrome/mozapps/plugins/plugins.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/plugins/plugins.dtd,v <-- plugins.dtd new revision: 1.6; previous revision: 1.5 done Checking in toolkit/locales/en-US/chrome/mozapps/plugins/plugins.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/plugins/plugins.properties,v <-- plugins.properties new revision: 1.9; previous revision: 1.8 done Checking in toolkit/locales/en-US/chrome/mozapps/profile/createProfileWizard.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/profile/createProfileWizard.dtd,v <-- createProfileWizard.dtd new revision: 1.6; previous revision: 1.5 done Checking in toolkit/locales/en-US/chrome/mozapps/profile/profileSelection.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/profile/profileSelection.dtd,v <-- profileSelection.dtd new revision: 1.4; previous revision: 1.3 done Checking in toolkit/locales/en-US/chrome/mozapps/update/updates.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/update/updates.dtd,v <-- updates.dtd new revision: 1.30; previous revision: 1.29 done Checking in toolkit/locales/en-US/chrome/mozapps/update/updates.properties; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/update/updates.properties,v <-- updates.properties new revision: 1.35; previous revision: 1.34 done Checking in toolkit/locales/en-US/installer/unix/install.it; /cvsroot/mozilla/toolkit/locales/en-US/installer/unix/install.it,v <-- install.it new revision: 1.4; previous revision: 1.3 done Checking in toolkit/locales/en-US/installer/windows/install.it; /cvsroot/mozilla/toolkit/locales/en-US/installer/windows/install.it,v <-- install.it new revision: 1.4; previous revision: 1.3 done
(In reply to comment #80) > attachment 290319 [details] [diff] [review]: > > Checking in … Cheers Reed, for finally getting this done! However, as I feared, you forgot the three en-US files in the obsoleted inspector patch. Could you please do them as well? I will try to handle the comments of Robert soon, and will then advance to doing the same thing for TB.
Was the first part of comment #76 ever split into a separate bug? It is an obvious discrepancy on the Help menu.
Argh. Thanks for screwing the ChatZilla locale files without even consulting us - none of the core team are CCed, none have commented, or even been asked AFAICT. Search and replace is NOT your friend. First of all, all the cmd.*.params changes have BROKEN their respective commands. Secondly, a lot of the changes outside of cmd.*.label are not used in menus/GUI elements, and really shouldn't have been changed without consultation first. I've a right mind to just back out the entire set of changes that were applied to ChatZilla, but I will at least discuss it with the other ChatZilla developers first. (There's also something wrong with ChatZilla being clobbered by a Firefox: General bug, of all things.)
I should note that the above comment implies to Venkman as well. This is pretty much sev: critical as far as I'm concerned, because ChatZilla and Venkman are now mostly unusable on trunk. Please either come up with a patch that fixes the params strings at least, or back out the changes to extensions/irc and extensions/venkman, and file a followup bug to deal with problems you see there.
(In reply to comment #83) > Argh. Thanks for screwing the ChatZilla locale files without even consulting us > - none of the core team are CCed, none have commented, or even been asked > AFAICT. > > Search and replace is NOT your friend. I explicitly stated above that the changes were done using search & replace. From the strings, it is unclear that that change would break anything. I find it strange that commands would depend from localizable strings. And it is/was impossible to test all of this since it were such massive changes. > First of all, all the cmd.*.params changes have BROKEN their respective > commands. If I see it right, search & replace will be our friend here: by simply reverting all [<…>] back to [<...>], 99% of the problems should be solved. Same thing for venkman. Sorry though.
(In reply to comment #85) > (In reply to comment #83) > > Argh. Thanks for screwing the ChatZilla locale files without even consulting us > > - none of the core team are CCed, none have commented, or even been asked > > AFAICT. > > > > Search and replace is NOT your friend. > > I explicitly stated above that the changes were done using search & replace. > From the strings, it is unclear that that change would break anything. I find > it strange that commands would depend from localizable strings. http://mxr.mozilla.org/seamonkey/source/extensions/irc/locales/en-US/chrome/chatzilla.properties#89 > And it is/was impossible to test all of this since it were such massive > changes. Then it would have been better if you'd stuck with your original plan of changing code that was tested (browser, mail, calendar), or at least alerted module owners/peers of the other things you were touching that you were going to do a mass change. Finally, it would be preferably to not lump all these changes together in a 300k patch and term that a "browser locale" patch, when it clearly isn't. I'm not sure why you changed your approach there anyway. Untested changes getting rubberstamped and checked in are bad. > > First of all, all the cmd.*.params changes have BROKEN their respective > > commands. > > If I see it right, search & replace will be our friend here: by simply > reverting all [<…>] back to [<...>], 99% of the problems should be solved. > > Same thing for venkman. You should probably use <...> and <…> for replacing - the [] braces indicate optionality, and there could be circumstances where they are not present (I don't think there are, though).Anyway, that just proves once more that search/replace without understanding the code you're changing is not really your friend... It would be smarter to look at your patch and revert the changes to .params, also because the <...> string could possibly occur in non-params strings. Slightly more work, sure, but smarter. > > Sorry though. > Thank you. Looking forward to that patch!
Blocks: 405866
Depends on: 405871
Attached patch backout changes to irc, fix inspector (obsolete) (deleted) — Splinter Review
Ok, after some trying out, I came up with this patch. It reverts the wrong changes to irc/. As I said, simply undid all the <…>, replacing them with <...>. Gijs already fixed Venkman (see dependant bug). Furthermore, the patch contains the four files in inspector/ which Reed forgot.
Attachment #290619 - Flags: review?(gijskruitbosch+bugs)
Attachment #290619 - Flags: review?(silver)
Comment on attachment 290619 [details] [diff] [review] backout changes to irc, fix inspector You can have r+ from me on the CZ backout, but I can't say anything about inspector, so asking review from sdwilsh for those bits.
Attachment #290619 - Flags: review?(gijskruitbosch+bugs)
Attachment #290619 - Flags: review?(comrade693+bmo)
Attachment #290619 - Flags: review+
Comment on attachment 290619 [details] [diff] [review] backout changes to irc, fix inspector r=silver on the ChatZilla changes. Further changes are required, however. Really stupid to include inspector changes in the same patch, though.
Attachment #290619 - Flags: review?(silver) → review+
Attached patch patch only for irc [checked in] (deleted) — Splinter Review
Ok, James is right, it was a bad idea to mix these things up. This is the patch only for ChatZilla.
Attachment #290619 - Attachment is obsolete: true
Attachment #290678 - Flags: review+
Attachment #290678 - Flags: approval1.9?
Attachment #290619 - Flags: review?(comrade693+bmo)
Attached patch inspector locales [checked in] (deleted) — Splinter Review
and this is the inspector part.
Attachment #290679 - Flags: review?(comrade693+bmo)
Comment on attachment 290678 [details] [diff] [review] patch only for irc [checked in] ChatZilla patches don't need approval. :-) I'll check this in tonight (assuming the tree is open and green etc.).
Attachment #290678 - Flags: approval1.9?
Comment on attachment 290679 [details] [diff] [review] inspector locales [checked in] rs=sdwilsh This needs approval to land.
Attachment #290679 - Flags: review?(comrade693+bmo) → approval1.9?
Attachment #290679 - Flags: approval1.9? → approval1.9+
Checking in extensions/irc/locales/en-US/chrome/chatzilla.properties; /cvsroot/mozilla/extensions/irc/locales/en-US/chrome/chatzilla.properties,v <-- chatzilla.properties new revision: 1.145; previous revision: 1.144 done Checking in extensions/inspector/resources/locale/en-US/inspector.dtd; /cvsroot/mozilla/extensions/inspector/resources/locale/en-US/inspector.dtd,v <-- inspector.dtd new revision: 1.13; previous revision: 1.12 done Checking in extensions/inspector/resources/locale/en-US/viewers/dom.dtd; /cvsroot/mozilla/extensions/inspector/resources/locale/en-US/viewers/dom.dtd,v <-- dom.dtd new revision: 1.15; previous revision: 1.14 done Checking in extensions/inspector/resources/locale/en-US/viewers/jsObject.dtd; /cvsroot/mozilla/extensions/inspector/resources/locale/en-US/viewers/jsObject.dtd,v <-- jsObject.dtd new revision: 1.5; previous revision: 1.4 done Checking in extensions/inspector/resources/locale/en-US/viewers/styleRules.dtd; /cvsroot/mozilla/extensions/inspector/resources/locale/en-US/viewers/styleRules.dtd,v <-- styleRules.dtd new revision: 1.7; previous revision: 1.6 done
Attachment #290319 - Attachment description: patch for browser locales, tested (unbitrotten) → patch for browser locales, tested (unbitrotten) [checked in]
Attachment #290678 - Attachment description: patch only for irc → patch only for irc [checked in]
Attachment #290679 - Attachment description: inspector locales → inspector locales [checked in]
(In reply to comment #82) > Was the first part of comment #76 ever split into a separate bug? It is an > obvious discrepancy on the Help menu. Done, bug 406083.
Depends on: 406083
Just to note taht Help > Report Web Forgery... ends with dot dot dot.
Depends on: 407481
(In reply to comment #96) > Just to note taht Help > Report Web Forgery... ends with dot dot dot. That is bug 406083.
No longer depends on: 407481
Depends on: 407481
Flags: wanted-firefox3+
Whiteboard: [wanted-firefox3]
These files still contains "...". /seamonkey/xpfe/communicator/resources/content/nsContextMenu.js /seamonkey//mail/base/content/nsContextMenu.js /seamonkey//suite/common/nsContextMenu.js Is it needed to repalce them like Bug 407481 ?
It may be needed to fix TruncateStringAtWordEnd() in /editor/ui/composer/content\editorUtilities.js
(In reply to comment #98) > These files still contains "...". > > /seamonkey/xpfe/communicator/resources/content/nsContextMenu.js > /seamonkey//mail/base/content/nsContextMenu.js > /seamonkey//suite/common/nsContextMenu.js Even more. In Trunk there's still more than 500 places where those tripple dots are used. Some of these have only recently been added (e.g. but patches tu bug #394392 or bug #193001). Here's a quick grep for three dots in en-US directories. It's 550 lines, and I don't think many comments have slipped into that file.
(In reply to comment #100) > Created an attachment (id=298932) [details] > Grep for three dots in a row in en-US directories > > (In reply to comment #98) > > These files still contains "...". > > > > /seamonkey/xpfe/communicator/resources/content/nsContextMenu.js > > /seamonkey//mail/base/content/nsContextMenu.js > > /seamonkey//suite/common/nsContextMenu.js > > Even more. In Trunk there's still more than 500 places where those tripple dots > are used. Some of these have only recently been added (e.g. but patches tu bug > #394392 or bug #193001). Here's a quick grep for three dots in en-US > directories. It's 550 lines, and I don't think many comments have slipped into > that file. > That's 80% mail/news (which is in the patch here but has bitrotted and was apparently never checked in) - and some IRC and Venkman (which was backed out for good reasons, see above comments). The toolkit and penelope things should probably get their own bug. Which was true for just about everything in that list, actually, given this bug is in Firefox > General...
SeaMonkey also still has the triple dots everywhere (suite/ and, as Gijs mentioned, mailnews/)
Attached file triple dots in *.js and *.cpp (deleted) —
This is result of searching triple dots in *.js and *.cpp in Firefox trunk7s source other than Comment, Extension, Test, Assertion, Log, Non-string ... (e.g. function declaration of C++). Some of them may be needed to fix.
(In reply to comment #103) > Some of them may be needed to fix. Right, some of them. And some of them not. Comments and debug console output should probably not be changed, see also comment #76
Depends on: 417807
For some reason Win 98 (NL, default) seems not to be able to display the (0x85) converted ellipsis very well for the installer and updater files Axel named in comment 49, or attachment 280210 [details] [diff] [review]; they look like a vertical bar. As the current/upcoming Sunbird releases are still able to run on Win 98, I suppose it would be best to exclude them from Sunbird (yet). For the same reason, taking them in for Mozilla 1.8 branch may be not a good idea either.
I thought all this was done for trunk only anyways, where Win98 is unsupported - with this cross-commit policy in calendar, I see why this can get difficult though. At some point, when the calendar team gets serious about trunk, this will be needed there for consistency though.
Attached patch this should be undone (deleted) — Splinter Review
This should not have been changed, as the comment says.
Attachment #306749 - Flags: review?
Attachment #306749 - Flags: review? → review?(peterv)
(In reply to comment #107) > This should not have been changed, as the comment says. As we have just been discussing this on irc, I tend to disagree. I presume this change handles both comment and a text string _about_ code, not code itself that could do any harm. It’s fine when the ellipsis will be left out here though (as there are many other comment lines that still use dots), but I see no reason not to leave it in.
Attached patch patch for mail [checked in] (deleted) — Splinter Review
Ok, another try for TB: tested, looks good (although a lot of other stuff doesn’t, but that’s trunk, I suppose).
Attachment #280205 - Attachment is obsolete: true
Attachment #306767 - Flags: review?
Attachment #306767 - Flags: review? → review?(bienvenu)
Comment on attachment 306767 [details] [diff] [review] patch for mail [checked in] r=me, thanks for the patch, and checked in. Please file any issues or missed strings as new bugs in the Thunderbird product; this bug was already a hopeless tangle six months ago.
Attachment #306767 - Attachment description: patch for mail → patch for mail [checked in]
Attachment #306767 - Flags: review?(bienvenu) → review+
Comment on attachment 306749 [details] [diff] [review] this should be undone I'm not really a good reviewer for this.
Attachment #306749 - Flags: review?(peterv) → review?(l10n)
(In reply to comment #105) > For some reason Win 98 (NL, default) seems not to be able to display the (0x85) > converted ellipsis very well for the installer and updater files Axel named in > comment 49, or attachment 280210 [details] [diff] [review]; they look like a vertical bar. As the > current/upcoming Sunbird releases are still able to run on Win 98, I suppose it > would be best to exclude them from Sunbird (yet). For the same reason, taking > them in for Mozilla 1.8 branch may be not a good idea either. Hm, strange. I just checked the installer of the Lithuanian build in English Windows 98 running in VMware. Works like a charm. Should I really revert those strings?
(In reply to comment #105) (In reply to comment #108) Trouble on ellipsis in updater's message is bug 405871. I think, updater of almost of products (e.g. Thunderbird, Sunbird) has same problem.
Comment on attachment 306749 [details] [diff] [review] this should be undone The rationale for this patch from comment 107 doesn't hold, thus r-. Not that I have a strong opinion on either side.
Attachment #306749 - Flags: review?(l10n) → review-
All patches appear to have landed, resolving FIXED. Please reopen ASAP if more work needs to be done here.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
(In reply to comment #117) > All patches appear to have landed, resolving FIXED. Please reopen ASAP if more > work needs to be done here. At this point, I think it'd be better to file a new bug.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: