Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#6721 closed bug (fixed)

Mail's reply preamble menu without "\n"

Reported by: humdinger Owned by: zooey
Priority: normal Milestone: R1
Component: Kits/Locale Kit Version: R1/Development
Keywords: Cc: jonas@…
Blocked By: Blocking: #7415
Has a Patch: no Platform: All

Description

This is 38987.

In Mail's preferences you can set a reply preamble (full name, date etc.). The newline would be "\n", but this isn't displayed in the pop-up menu. The source line is

B_TRANSLATE("\\n - Newline"),

Change History (16)

comment:1 Changed 9 years ago by pulkomandy

Looks like a bug somewhere in the localization system. Is the
n showing up properly in the catkeys files ?

comment:2 Changed 9 years ago by humdinger

The line of the German catkeys says

\\n - Newline	Mail		\\n - Zeilenumbruch

So with extra excaping "\", I'd say that is OK, too. Only the actual pop-up menu in Mail's preferences panel won't display the "\n". Switching the locale to French, won't show it either.

comment:3 Changed 8 years ago by humdinger

Blocking: 7415 added

(In #7415) Doppelgänger. :)

comment:4 Changed 8 years ago by czeidler

Cc: jonas@… added

comment:5 Changed 8 years ago by jonas.kirilla

I'm guessing the issue is the escapeQuotedChars() function in this file being too simplistic

http://haiku.it.su.se:8180/source/xref/src/tools/locale/PlainTextCatalog.cpp#57

which is used by linkcatkeys (to create catalog files from catkey files)

http://haiku.it.su.se:8180/source/xref/src/tools/locale/linkcatkeys.cpp#101

and the plaintext catalogs strings fed to a more complex parseQuotedChars() function in the hashmap catalog

http://haiku.it.su.se:8180/source/xref/src/kits/locale/HashMapCatalog.cpp#147

comment:6 Changed 8 years ago by pulkomandy

Right, there is a bug in the first one :

stringToEscape.ReplaceAll("\\","\\\\");
stringToEscape.ReplaceAll("\n","\\n");

The first line will do something like this : "\\n" > "\\\\n"

and the second one afterwards : "\\\\n" > "\\\\\n"

Definitely not what we want...

Not sure how to fix it, maybe it will get as complex as the first one.

The problem is as follow :

  • The key for each string is computed at runtime, so it's done from the string after compilation, where \n has been replaced by a real newline and so on
  • So, linkcatkeys must compute the key from the escaped string.
  • On the other hand, the catkey files must not be unescaped, because newlines and tabs are part of the file syntax here.

parseQuotedChars works fine, as far as I can tell. It was tested a lot more.

Last edited 8 years ago by pulkomandy (previous) (diff)

comment:7 Changed 8 years ago by jonas.kirilla

Resolution: fixed
Status: newclosed

Workaround committed in hrev41158.

comment:8 Changed 8 years ago by axeld

Resolution: fixed
Status: closedreopened

Reopened as the underlying problem has not been fixed yet, and there is no other ticket to track it either. Please take more care not to bury existing bugs this way.

comment:9 Changed 8 years ago by axeld

Component: Applications/MailKits/Locale Kit

comment:10 Changed 8 years ago by jonas.kirilla

I think the problem appears in the first position

"\\n"

but not in a following one

" \\n"

The function parseQuotedChars() in http://haiku.it.su.se:8180/source/xref/src/kits/locale/HashMapCatalog.cpp#147

assumes the initial character isn't a backslash, when it sets

bool quoted = false;

I think it should be something like this

bool quoted = in[0] == '\\';

This, however, changes the fingerprints and breaks the build in a massive way. (And HTA?)

I hand-edited the fingerprints of the catkey files for catalogs that wouldn't build, to be what the build stated as expected, but on a subsequent build I seemed to be back on square one, having to edit files manually again. So I don't know how to approach it.

comment:11 Changed 8 years ago by pulkomandy

There is another instance of the function which is used at build time. IIRC in src/tools or src/build. This is because at build time the full-blown locale kit is not available and collectcatkeys/linkcatkeys must work on a failsafe basis.

You need to make change to both to keep them in sync, and it should be fine.

comment:12 Changed 8 years ago by jonas.kirilla

I can't find another parseQuotedChars() function or another BHashMapCatalog.

comment:13 in reply to:  10 Changed 8 years ago by zooey

Owner: changed from bga to zooey
Status: reopenedin-progress

Replying to jonas.kirilla:

The function parseQuotedChars() in http://haiku.it.su.se:8180/source/xref/src/kits/locale/HashMapCatalog.cpp#147

assumes the initial character isn't a backslash, when it sets

bool quoted = false;

I think it should be something like this

bool quoted = in[0] == '\\';

No, the initial character can't be quoted, since there's no preceeding backslash. The initial character can be a backslash, of course, but that would quote the following character - that part of the code seems to be just fine.

I'll dig in a bit more ...

comment:14 in reply to:  12 Changed 8 years ago by zooey

Replying to jonas.kirilla:

I can't find another parseQuotedChars() function or another BHashMapCatalog.

That's because the one pulkomandy was mentioning has been removed in 40527, such that now both the standard collectcatkeys/linkcatkeys as well as the build versions use the same implementation of BHashMapCatalog.

comment:15 Changed 8 years ago by zooey

Resolution: fixed
Status: in-progressclosed

Fixed in hrev41167. The underlying problem was that linkcatkeys did an unintended unescaping of quoted chars when collecting all the individual catkey-files into the target catalog.

comment:16 Changed 8 years ago by jonas.kirilla

Thanks!

Note: See TracTickets for help on using tickets.