Closed Bug 21418 Opened 25 years ago Closed 24 years ago

Support "\n" in string bundle strings to force line breaks in dialog text

Categories

(Core :: Internationalization, defect, P3)

Product:
Core
Component:
Internationalization
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: sujay, Assigned: ftang)

References

Details

(Whiteboard: [nsbeta2+] reviewed fix in hand. Wait tree open to check in.) 

using 12/10 build of apprunner (1999121008)

1) launch apprunner
2) launch editor
3) type some text
4) close the window by clicking on upper right hand X in corner

notice the panel that comes up:

"Save changes to "untitled" nbefore closing"

delete the "n" in front of the word "before"

all platforms.
Status: NEW → ASSIGNED
Summary: mispelled save panel on exit → Support "\n" or "<b>" in string bundle strings to force line breaks in dialog text
Target Milestone: M14
This is a "\n" embeded in the string, which is supposed to be translated into
a line break. Unfortunately, promises to support this were not fulfilled by
XPFE team, so I will look into implementing it myself.
I changed the summary to reflect this task.
*** Bug 22548 has been marked as a duplicate of this bug. ***
Assignee: cmanske → trudelle
Status: ASSIGNED → NEW
this really needs to go to xpfe team so implementation follows their process.
Assigning this to trudelle
Target Milestone: M14
forgot to clear milestone
Charlie, who promised this?
Assignee: trudelle → rgoodger
assigning to Ben Goodger for consideration post-beta1
Status: NEW → ASSIGNED
My method of fixing this is more of a hack than anything else, but here it is:

var string = bundle.GetStringFromName("foostring");
string = string.replace(/\s*<html:br\/>/, "\n");
..

I'll integrate this into the new set of Chrome common dialogs  I'm planning to
make. Everywhere else where you need to use this, please just use regular
expressions to detect a delimiter. (In my case I'm just embedding <html:br/>
into the strings in properties files...)
Your hack is fine for JS. We also need this to work when using strings in C++.
See nsEditorShell.cpp for examples where we launch messagebox-like dialogs
from C++ using the nsICommonDialogs interfaces. So some work needs to be done
in those implementations to also handle embedded "\n".
hm.. in that case, do you think we should push this over to an i18n person so 
this can get fixed "properly"?

who owns the string bundle code?
True, I18N does need to be consulted. I don't know who owns string bundles.
m14 radar
Target Milestone: M14
*** Bug 23719 has been marked as a duplicate of this bug. ***
ok, this is a back end problem :)
Assignee: ben → tao
Status: ASSIGNED → NEW
Component: Editor → Internationalization
The problem is more likely to be in the stream converter which trims down the 
'\' character during parsing. To get the around the problem in C++ code, you
might want to split the string into two segments and concatenate it on the fly.

However, why would we use '\n' to format the string in dialog in the first
place? I thought this shall be done with HTML tags or style setting.
because some of the strings we retrieve are passed into alert() etc dialogs 
which expect (by specification) strings with newline breaks like "\n" etc. 
Why we need to be able to process embeded "\n":
A string that is a single message for a message dialog cannot be broken into
multiple strings - there's just one parameter in the "common dialog" interface
that represents the message. And we need to format such a message, when it is
long, into predictable "paragraphs" by embeding line breaks.
We don't want to make the string user parse such a string as HTML, i.e., require 
understanding of things like <br>. It would be great if we could get at DTD
strings in C++, but we can't, thus we have the "string bundle" solution.


I agree that we shall not break a complete sentence into segments; it makes
translation work harder.

I also agree that StringBundle module shall parse escape characters properly.

However, I have reservation about the need of using "\n" to format a 
long paragraph or sentence since when such string is translated into languages
other than the original one it is written, the "\n" formatted might break on
the wrong position.

If the "common dialog" is an XP widget, what is preventing us from using 
CSS to format it?  Is this because alert() dialog is a native widget and 
HTML layout engine can't wrap long line nicely? 

Any pointer to the spec I can look at? I probably miss something here :-)
CSS doesn't help at all. This issue is not that a long string won't wrap when
displayed in the dialog, it's that we want to *control* where it wraps.
E.g.: We form a string that tells the user to save a file. We build the string
like this:
Save changes to:
file:///somelongdirectoryname/users/myname/myfile.html

We want to be sure the "file..." part starts on a new line, so we need to 
force a break after "to:"
As to embeding "\n" in strings in other languages, I can't help. It seems that
problem needs to be solved by the translators (localizers), no?
I don't know of any spec relating to this issue. But we were able to do this
with the XPStrings that we used in 4.x and earlier products.
I see. In this case, you probably want to add "LOCALIZATION NOTES" to the 
localizers so that they know how to translate this string. In some language,
the wording of the example entered in this bug report:

	"Save changes to "untitled" \nbefore closing"

You want it to look like

	Save changes to 
	"untitled" 
	before closing

But, after translated, it might look like

	Before closing, save changes to 
	"untitled" 

We need to add LocNote to this file. CC fergus.
Per the discussion in i18n meeting, I am reassigning this bug to Frank who owns 
stream converter.
Assignee: tao → ftang
Status: NEW → ASSIGNED
fix and check in
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
I updated to get your fix, but now I see the "\n" embeded in the message
when I try to close the window after making changes to an existing file,
as described above. So it seems part 2 of the fix is to make the common dialog
code that uses the string parse it to search for "\n" and replace them with
<br> in the dialog.
Ben: Can you do that part?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
tao- I fix the converter part and now \n will convert to \n . Reassign to tao 
for the remaining issues
Assignee: ftang → tao
Status: REOPENED → NEW
Summary: Support "\n" or "<b>" in string bundle strings to force line breaks in dialog text → Support "\n" or "<br>" in string bundle strings to force line breaks in dialog text
The remaining issues are not stringbundle related; rather, they are of dialog 
implementation.

Pass to Ben
Assignee: tao → ben
I hate to break this to you all but the strings I'm getting dont actually 
contain "\n"s, they contain "\" and "n", the string equivalent being "\\n". I 
don't want to have to special case this in the common dialog code because a) I 
dont know how I can tell the cdialog that this is a string from a string bundle 
and to treat \\ns as \ns, and I shouldnt have to, as someone may want to 
legitimately place the string "\n" in a dialog, and me parsing that out as "\n" 
would ruin that. 

reassigning to ftang as he is the one who attempted to fix this :)
Assignee: ben → ftang
Ben- I am NOT the person who attempt to fix the whole problem. I simply fix the
bug in the unicode converter which convert "\n" to "n" and make it convert to
"\n" again. The unicode converter have no assumption about how people use it.
You are welcome to reassign the bug to me if this is a conversion problem. Also,
I am very confused about your last comment. What is your input and what is your
expect output ?
Assignee: ftang → ben
Hi, Ben:


Are you looking for 

	1. a string "\n" which contains a "\" followed by "n" or 
	2. a newline character '\n' ?

Thanks.
We want a single newline character, not '\'+'n'
...what cmanske said...
also, please do not 'special case' the "\n" I should also be able to do \t, \b, 
etc, anything allowed in javascript. (refer to JavaScript: The Definitive 
Reference for a full list) 
Hi, Frank:

Please refer to this excerpt from Java Language Spec:

"Within the element string (but not the key), the ASCII escape sequences 
\t, \n, \r, \\, \", \', \ (a backslash and a space), and \uxxxx are 
recognized and converted to single characters. Moreover, if the last 
character on the line is \, then the next line is treated as a
continuation of the current line; the \ and line terminator are simply
discarded, and any leading whitespace characters on the continuation line 
are also discarded and are not part of the element string."

Reassign to Frank to this part.
Assignee: ben → ftang
1. The following is easy \t, \n, \r, \\, \", \'
2. what should \ (a backslash and a space) convert to ?
3. I don't think the following should be done in the converter level- if the 
last 
character on the line is \, then the next line is treated as a
continuation of the current line; the \ and line terminator are simply
discarded, and any leading whitespace characters on the continuation line 
are also discarded and are not part of the element string.

The first problem is what is a "line terminator" ? DOS favor ? UNIX favor ? or 
Mac favor ?

I neither feel comfortable to remove white space in "unicode converter"

I think we make the first mistake to make this function inside a converter. This 
functionality should really belong to a scanner / lexer....
Status: NEW → ASSIGNED
IMO, while  1) should be fixed in the ocnverter,  2) and 3) should be in the lexer or parser  as frank suggested.
please notice 26938 got introduced when I first change the mapping "\n" => "n" 
to "\n" => "\n"

I think the conversion for \n , \t , \" , \\ , \' and the end of line \ should 
be take care in the scanner in nsPersistentProperties::Load(nsIInputStream *aIn)
or higher level. 
Erik, what is your opinion ? 
Frank, as you and I just discussed, our Properties class should behave the same
way as Java's, for consistency and shareability of files. \n \r \f \t \uXXXX
should all map to the appropriate Unicodes.
The following patch should convert "\t" to '\t' ,"\r" to '\r', "\f" to '\f', 
"\n" to '\n', and ignore any \x (where x is other characters) to x .

Please try it and check in after testing. Then reassing this bug to ben so he 
can scan for '\n' and replace to "<BR>" 

Index: nsPersistentProperties.cpp
===================================================================
RCS file: /m/pub/mozilla/xpcom/ds/nsPersistentProperties.cpp,v
retrieving revision 1.2
diff -c -r1.2 nsPersistentProperties.cpp
*** nsPersistentProperties.cpp  2000/01/06 23:02:32     1.2
--- nsPersistentProperties.cpp  2000/02/08 20:12:04
***************
*** 133,139 ****
              c = SkipWhiteSpace(c);
            }
            else {
!             value.Append('\\');
            }
          }
          value.Append((PRUnichar) c);
--- 133,154 ----
              c = SkipWhiteSpace(c);
            }
            else {
!             switch(c) {
!                case 't':
!                   value.Append('\t');
!                   break;
!                case 'n':
!                   value.Append('\n');
!                   break;
!                case 'r':
!                   value.Append('\r');
!                   break;
!                case 'f':
!                   value.Append('\f');
!                   break;
!                default:
!                   break;
!             };
            }
          }
          value.Append((PRUnichar) c);
Assignee: ftang → tao
Status: ASSIGNED → NEW
tao- forget about the patch. It will intrdouce more problem.
Assignee: tao → ftang
the issue is much complex than we thought. We cannot convert "\n" to '\n' in the 
converter level. Otherwise the nsPersistentProperties scanner will be confused. 
It seems we have to do the conversion in the scanner level.
Status: NEW → ASSIGNED
I think my patch in nsPersistentProperties.cpp is probably a fix
Depends on: 26938
I believe we should do the following to fix both 21418 and 26938-
1. the \n decoding cannot be done in the converter but scanner, otherwise, the 
scanner will be confused with the REAL line terminator form this new line escape
2. the \n and \\ and \uXXXX have to be decode in the same place so there won't 
be conflict. 

So, I suggest we do the following
1. Do not use x-u-escaped converter in nsPersistentProperties.cpp, instead, use 
ASCII or UTF-8 converter, or even read the raw bytes
2. Put the the \n, \f, \r, \t, \uXXXX decoding logic into  
nsPersistentProperties.cpp scanner
3. Make sure "x-u-escaped" decoder and encoder convert roundtrip.
both ben and tao think this is not a beta1 feature. Change it to M16.
Target Milestone: M14 → M16
Keywords: beta2
reassign to bobj
Assignee: ftang → bobj
Status: ASSIGNED → NEW
I'll take a look.
Assignee: bobj → tao
I still think this a good feature. Now that the message dialogs are 
smaller and you can see how long messages wrap, and how being able to 
control where we break will really help readability. For example, when showing
a message asking user to save a file:
Without break control:
Save changes to "Quarterly Report
for fall, 1999" before closing?
With break control:
Save changes to 
"Quarterly Report for fall, 1999" 
before closing?
The later scans better, don't you think?
We don't need to (or want to, for security reasons) support embeded HTML tags,
so let's not use "<br>". "\n" (those 2 characters, not the escaped value) 
is what we used to use, but you might want to use some other signal, such as
"%br%" embeded in the string. That pattern is what we use in Composer strings
to represent a variable that will be replaced with a different string.
For example, the string for the save file message is currently:
SaveFilePrompt=Save changes to "%title%" %reason%? 
where %title% is replaced with the Page Title and %reason% is another string.
So localizers should be already used to that pattern. This would be the string
with break control:
SaveFilePrompt=Save changes to%br%"%title%"%br%%reason%?
where %br% signals breaks in the string. Obviously variables inside %% shouldn't 
be localized.
Summary: Support "\n" or "<br>" in string bundle strings to force line breaks in dialog text → Support "\n" in string bundle strings to force line breaks in dialog text
Status: NEW → ASSIGNED
The root cause of this bug is that the x-u-escaped converter eats up all '\'
in escaped characters such as '\n' and '\r'. We shall fix this problem first.
In other words, when "\n" or "\r" appears in the value part of the proeprty
files, they should be converted into their corresponding UCS-2 characters.
We shall follow Java property file's syntax instead of inventing our own rule.

-- BEGIN excerpt for reference ---

Within the element string (but not the key), the ASCII escape sequences \t, \n, 
\r, \\, \", \', \ (a backslash and a space), and \uxxxx are recognized and 
converted to single characters. Moreover, if the last character on the line is 
\, then the next line is
treated as a continuation of the current line; the \ and line terminator are 
simply discarded, and any leading
whitespace characters on the continuation line are also discarded and are not 
part of the element string.

-- END excerpt for reference ---

Application shall then look for '\n' (in ucs-2) as linefeed to break the 
string.

I will correct the conversion of "\n" to '\n" first.
Keywords: nsbeta2
Putting on [nsbeta2+] radar for beta2 fix.
Whiteboard: [nsbeta2+]
M16 has been out for a while now, these bugs target milestones need to be 
updated.
This problem have to be fixed in pair with 26938. Can we live without this in 
PR2? tao, please consider remove [nsbeta2+] for this. 
Tao - if you don't fix 21418 prior to nsbeta2 please ensure that it is fixed 
FIRST THING in the nsbeta3 cycle! This feature has already slipped one beta and 
it's a rather bad problem if you ask me ;) 

There are many places where we want to use newlines in dialogs, creating 
nonstandard entities to do so is somewhat hacky. 


Hi, Frank:

I agree with Ben that the hacky way is really awkward. Could you take a look
and see if we can resolve it in PR2?
Assignee: tao → ftang
Status: ASSIGNED → NEW
ben- I will talk to you next week and figure out the basic requirment.
Status: NEW → ASSIGNED
The fix is post in http://warp/u/ftang/tmp/fix21418.txt
tao- please review.
Whiteboard: [nsbeta2+] → [nsbeta2+] fix in hand. Wait for review
Hi, Frank:


The patch looks good. One question:

  * You use UTF-8 instead of x-u-escaped converter. Are you using utf8 
    converter to convert the escape sequence into single byte unicode (for 
    example, "\n" into '\n')?

One comment: have you tested the patch to see if it fix 26938, too?

Interesting approach!



Thanks
1. we cannot use x-u-escaped converter becaues the \n and \uxxxx have to be take 
care in the same time. If we use the x-u-escaped converter, then we cannot 
handle \n correctly. I can use ISO-8859-1 or UTF-8. I choose UTF-8 in this case.

>Are you using utf8 converter to convert the escape sequence into single byte 
unicode (for example, "\n" into '\n')?

The answer is NO. Both \n and \uXXXX is a lexical issue of the scanner. It HAVE 
to be implement in the lexical level but not in the encoding level.


No, this does not fix 26938. However, since the fix remove the usage of 
x-u-escaped from nsPersistenceProperty, it will simplified the fix for 26938.
Whiteboard: [nsbeta2+] fix in hand. Wait for review → [nsbeta2+] reviewed fix in hand. Wait tree open to check in.
No longer depends on: 26938
swap the dependency. make this bug block 26938 instead. We need to check in the 
fix for this one first then check in the fix for 26938.
Blocks: 26938
fix and check in
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 24 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
verified in 7/12 build.
You need to log in before you can comment on or make changes to this bug.