433976 - [nl] Dutch should use proper double quote symbols

Status: RESOLVED FIXED

(Mozilla Localizations :: nl / Dutch, defect)

Description

[Hendrik Maryns]

Attachment: patch die enkel aanhalingstekens vervangt

Zoals besproken via irc met Ton, willen we, na de enkele aanhalingstekens, ook dubbele gekrulde aanhalingstekens invoeren.  Ik heb hiervoor een patch gemaakt.  Sorry dat het zo lang geduurd heeft.

Comment 1

[u81239]

Good idea. Which curly quotes do we use, “bla” or „bla”?

~Grauw

p.s. if you want an easy way to enter those symbols, check out my custom keyboard layout (an extension of US international) for Windows at http://www.grauw.nl/blog/entry/430 .

Comment 2

[Hendrik Maryns]

(In reply to comment #1)
> Good idea. Which curly quotes do we use, “bla” or „bla”?

Het gaat om Nederlands, dus het eerste.  Het tweede is, VZIW enkel nog in het Duits gebruikelijk.  Ja had even naar de patch kunnen kijken.

Comment 3

[u81239]

Wel, ik heb op de basisschool de tweede geleerd :). Dat was dus de vraag, welke correct is.

Het was me overigens niet opgevallen dat er een patch was.

Comment 4

[Tim Maks van den Broek [:mad_maks]]

quote van Pike in de nieuwsgroep:
Cut-off time for any landings is going to be mid-day PDT tomorrow, so make sure that anything that you find that's *really* bad is getting my attention, in particular, I should be on CC and the patches should request approval1.9.

As always, we're going to be taking more fixes in 3.0.1, so there's no need to rush nits or code clean up or the like. 

deze patch ga ik dus onder zijn aandacht brengen na Fx3

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 321181 [details] [diff] [review]
patch die enkel aanhalingstekens vervangt

The patch is looking fine, but 3.0.1 just closed.

Putting this on the line for 3.0.2, I'll go through those approvals once the tree opens again.

Comment 6

[Axel Hecht [:Pike]]

Comment on attachment 321181 [details] [diff] [review]
patch die enkel aanhalingstekens vervangt

a=me, please land this with a check-in comment referencing this bug and my
approval. Use the fixed1.9.0.2 and verified1.9.0.2 keywords to track landing
and testing.

Comment 7

[Axel Hecht [:Pike]]

Does this still need to land?

Comment 8

[Ton [:Tonnes]]

(In reply to comment #7)
> Does this still need to land?
> 
It would be nice, but I have some doubts..

- I’m not sure whether Hendrik has tested these changes thoroughly yet, i.o.w. for all products, as the patch includes the entire source (Hendrik?)
- some changes may be obsolete or fail as a result of other edits (a.k.a. in Calendar, Suite, Mail)
- there has been some discussion about whether or not (rather not) to add quotes in some cases where (as in English) they don’t exist, like e.g. in dom.properties
- Suite will soon get a number of patches containing lots of more changes, so these changes might be left out
- maybe it’s wise to create new separate patches for different products in this bug and ask for split reviews, for different reasons? I guess the Firefox part can pass pretty soon, as long as notes 1 and 3 are taken into account

Comment 9

[Tim Maks van den Broek [:mad_maks]]

i will land the patch (firefox part) at the end of this week when i am back from my holiday

Ton why do do speak out so late this patch is here for more the 3 months ....

Comment 10

[Hendrik Maryns]

(In reply to comment #8)
> (In reply to comment #7)
> > Does this still need to land?
> > 
> It would be nice, but I have some doubts..
> 
> - I’m not sure whether Hendrik has tested these changes thoroughly yet,
> i.o.w. for all products, as the patch includes the entire source (Hendrik?)

Building…

> - some changes may be obsolete or fail as a result of other edits (a.k.a. in
> Calendar, Suite, Mail)

I updated my wc and am testing a new patch which is up tro date (and contains some missed "s)

> - there has been some discussion about whether or not (rather not) to add
> quotes in some cases where (as in English) they don’t exist, like e.g. in
> dom.properties

That is independent of the question which signs to use.  in case we add more quotes, they simply follow the rest of the code.  the upcoming patch only addresses existing quotes.

> - Suite will soon get a number of patches containing lots of more changes, so
> these changes might be left out

That might give hunks and require a new patch (from either side) but I don’t see a problem in that.

> - maybe it’s wise to create new separate patches for different products in
> this bug and ask for split reviews, for different reasons? I guess the Firefox
> part can pass pretty soon, as long as notes 1 and 3 are taken into account

Should I?  Just say so.  (Though I’d say this is something which justifies an nl-based patch, notwithstanding my learning trouble of keeping products separated ;-))

So Maks: wait a little more!

Comment 11

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 321181 [details] [diff] [review]
patch die enkel aanhalingstekens vervangt

ok, i made the patch obsolete, please keep in mind that next friday the gate closes for 3.0.2

Comment 12

[Ton [:Tonnes]]

(In reply to comment #10)
> 
> That is independent of the question which signs to use.  in case we add more
> quotes, they simply follow the rest of the code.  the upcoming patch only
> addresses existing quotes.

It wasn’t about _what_ signs to use, only about adding them, so that should be OK.

> That might give hunks and require a new patch (from either side) but I don’t
> see a problem in that.

As mentioned on irc yesterday, you could leave them out for SM as the 1.1 language pack already contains them and thus upcoming patches (that will use this for reference) would take them along. But if you insist on doing so, please do so in the SM bug.

> Should I?  Just say so.  (Though I’d say this is something which justifies an
> nl-based patch, notwithstanding my learning trouble of keeping products
> separated ;-))

Please do. I’d like to see the Calendar changes in the Calendar bug as well. If not, that would mean a trunk only change, so there’s another reason.. ;)

I.o.w., it’d be nice to see 2 patches here (for FF and TB), although I wouldn’t mind if they combine to 1, as long as you are sure TB has no problems.

Question (Pike?): what about the " characters? So far they haven’t changed for any locale and thus remain straight. I hope you don’t mean these when talking about the missed ones, and would like to get some things clear about whether or not these are allowed to change. That would mean a separate patch, I guess.

Comment 13

[Hendrik Maryns]

Attachment: patch for firefox and all common stuff

As requested by Ton, I’m splitting up the patches.  I’ll keep them in this bug though, it’s easier for keeping track.

A new patch for FF and all core components.  Tested superficially, seems fine.

Asking new review, since I also replaced all occurrences of ".

Comment 14

[Hendrik Maryns]

Attachment: patch for sunbird

only a few changes for SB

Comment 15

[Hendrik Maryns]

Attachment: patch for suite

This one might get obsolete when Ton’s changes get through.  Attaching anyway for reference.  To be checked later.

Comment 16

[Hendrik Maryns]

Attachment: patch for mail

And TB.  Unable to test this, since my build doesn’t work.

Comment 17

[Ton [:Tonnes]]

(In reply to comment #13)
> Created an attachment (id=331972) [details]
> patch for firefox and all common stuff

This is probably not the patch you intended to attach, but a new version for the one in bug 438556, in which you appear to have ignored some comments. Please attach a revised version there, not containing the extra quotes (OK etc.) and without the ‘Het’s in dom.p. I do approve about the comma and en dash however, and think it may be wise to add a comment about not using this type of dash here (at the end of existing comment lines) for future reference. en-US should do something similar I think, in favour of other locales.

And attach the proper FF+core patch here of course, but do not add any " changes (the localization notes are not there for nothing), as mentioned in comment 12. Unless Axel can confirm " signs are not really in for a good reason and thus 100% safe to be eliminated, but I would recommend to make a separate patch for those anyway.

Comment 18

[Ton [:Tonnes]]

(In reply to comment #14)
> Created an attachment (id=331973) [details]
> patch for sunbird

<!*censored>
This is the 3rd (!) time you change these files while definitely must now by now they cause trouble and have been changed back earlier as a result of your action. I’m not telling this again. If your memory lacks, keep track of revision/bug history!

And I’m not sure what is worse, but you even fail to patch the proper 2 files that contain the quotes to be changed :(

Comment 19

[Ton [:Tonnes]]

(In reply to comment #15)
> Created an attachment (id=331974) [details]
> patch for suite

Not allright. Contain discussed no-go’s for mobile phone, quot changes and such. It also seems by now you’re attaching patches for other bugs you filed recently.. :(

Comment 20

[Ton [:Tonnes]]

(In reply to comment #16)
> Created an attachment (id=331977) [details]
> patch for mail

Similar as above, not ok. Contains unnecessary and discussed (no-go) changes, extra quotes, and belongs to another bug. And navigator.properties (not flawless) isn’t needed..

Please try to work a little more secure, this may reduce workload for others.

Comment 21

[Hendrik Maryns]

Attachment: second try for SM

Sorry guys, I seem to have made all four patches above from the wrong working copy.  New ones follow.

Comment 22

[Hendrik Maryns]

Attachment: second try for TB

Comment 23

[Hendrik Maryns]

Attachment: second try for SB

Comment 24

[Hendrik Maryns]

Attachment: second try for FF

Note that all remarks from above apply for these patches.

Comment 25

[Hendrik Maryns]

Attachment: TB also includes editor

The subdirectory ‘editor’ belongs to Thunderbird, therefore this is the proper patch for TB.

Comment 26

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 332060 [details] [diff] [review]
second try for FF

please don't ask for a review of pike before we had a review!

Comment 27

[Hendrik Maryns]

(In reply to comment #26)
> (From update of attachment 332060 [details] [diff] [review])
> please don't ask for a review of pike before we had a review!

I don’t really understand this, you mean internal review first?  Fine.

Anyway, to take away doubts about ":

[13:24]	<Pike>	Hamaryns: the "don't translate &quot;" means just that you shouldn't do "&zitierzeichen;"
[13:25]	<Pike>	as long as you replace the &quot; with something that's not reserved char by xml specs as '"' is, you're fine

Comment 28

[Axel Hecht [:Pike]]

Comment on attachment 332060 [details] [diff] [review]
second try for FF

One question, did you verify that the installer still works, i.e., that the new quotes in browser/installer/custom.properties are in the code page that the installer uses?

Tim, it'd help if you made a real comment when doing the review and asking for approval. Stuff like risk, impact, testing coverage help me in not having to ask more questions :-)

Comment 29

[u81239]

(In reply to comment #11)
> please keep in mind that next friday the gate closes for 3.0.2

Comment 30

[Ton [:Tonnes]]

(In reply to comment #27)

Hendrik,

I’m not very happy with the fact you (as expected, to be honest) rushed from irc chat to this bug in order to post the quote above rightaway to get your point proven, prior to read my comments following them. Same for the fact you dared to ask anyone else’s review that morning within the channel while you probably had read here earlier this is not the way to act. To make it clear once more: I did not say quotes would cause trouble, but just asked you to make sure they don’t, which is why telling things like “Tonnes thinks this will give trouble because...” isn’t really true. Whether or not the proposal of introducing the quotes was originated by me or not, I just want (you) to make sure they don’t cause trouble (as I’ve seen they can definitely do so in SM) and I don’t want you to mess things up. I don’t think you or anyone else can blame me for doing so. And please don’t start about ‘personal feuds’ on irc. If you feel there are any, let me tell you this surprises me, but we could talk that over in the future and not within this or any other bug.

Nevertheless, I tested the patch as far as I could and think there won’t be much trouble introducing the curlies, but I can’t be sure for the quotes in custom.properties. The only way to check seems to run iconv on them and I wasn’t able to test that as I’ve not set up a build config to do so. Hendrik appears to have tested his build, but I’m not sure if that includes the installer. So I guess we have a choice to try and see (as Hendrik is out for 3 weeks), to have this tested by somebody else or eliminate the change on this file only, which is probably for to Tim Maks to decide..

Comment 31

[u81239]

Not again.

Ton, your comments 18, 19 and 20 weren’t exactly friendly, and neither is this new one. Everyone here is a volunteer, please keep an attitude that respects your fellow translators.

Comment 32

[Axel Hecht [:Pike]]

Approval is still pending on some off-line testing of custom.properties.

Comment 33

[Tim Maks van den Broek [:mad_maks]]

(In reply to comment #32)
> Approval is still pending on some off-line testing of custom.properties.
> 

Pike will you test custom.properties or do i need to do some thing? And if you approved the patch do you apply it?

Comment 34

[Axel Hecht [:Pike]]

Nah, I was expecting the nl team to do both the testing and the landing.

Comment 35

[Axel Hecht [:Pike]]

Comment on attachment 332060 [details] [diff] [review]
second try for FF

Taking off the .2 queue, please request approval for 1.9.0.3 when tests are done.

Comment 36

[Axel Hecht [:Pike]]

Comment on attachment 321181 [details] [diff] [review]
patch die enkel aanhalingstekens vervangt

Actually denoting the non-landing of this patch by putting in a a-.

Comment 37

[Ton [:Tonnes]]

Attachment: second try for FF v2

I’ve tried to do some iconv testing on custom.properties but with no luck, as the iconv used does not recognize the cp1252 charset. Despite that, I noticed some other locales use utf8 encoded double quotes in this file, so if they work I expect no problems for nl.

Nevertheless, I noticed the “OK” string concerned is the only one using the quotes, as opposed to all other OK occurrences in this and other installer files, as well as other files in the tree (as said, we hardly use quotes in button clicking texts). In other words, this could be seen as a small error in the English text. Therefore I suggest to eliminate the quotes here in order to keep consistency, just like e.g. German did.

The patch attached is att 332060 that contains only this change. The same applies to Calendar, Thunderbird and Suite of course, so I suggest to act similarly for those.

Comment 38

[Ton [:Tonnes]]

Attachment: second try for SB v2

Same as above for SB. Curlies on error console tested, eliminated on custom.properties. May be in time for 0.9 if all agree.

Comment 39

[Axel Hecht [:Pike]]

Ton, your arguments are not working, sorry.

iconv does support cp1252, as you can clearly see in the build logs, i.e., http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-l10n-nl/1221470520.1221479452.5585.gz&fulltext=1

iconv -f UTF-8 -t CP1252 

Other locales using utf-8 code points is worthless unless you specify which code page they're using.

Comment 40

[Ton [:Tonnes]]

(In reply to comment #39)
> 
> iconv does support cp1252, as you can clearly see in the build logs, i.e.,

Sorry, may be I should have said ‘the iconv _I_ used does not support..’. I know it should be supported for building FF, but it’s not within the DSL linux I tried this on. Just meant I wasn’t capable of testing it using iconv.

> Other locales using utf-8 code points is worthless unless you specify which
> code page they're using.

True. However, it’s the sv locale I was referring to, which I believe uses CP1252 as well (at least that is what I read from charset.mk). Must admit I checked that just a few minutes ago. ;)

Comment 41

[Ton [:Tonnes]]

Attachment: second try for SB v3

When looking closer to the ‘bij’ vs. ‘van’ inconsistency in calendar.properties, I noticed ‘uitschrijven’ does not fit with ‘aanmelden’. Thus changing this to ‘afmelden bij’.

Comment 42

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 338734 [details] [diff] [review]
second try for SB v3

Applied on the trunk / MOZILLA_1_8_BRANCH / SUNBIRD_0_9_BRANCH 

(but to late to get it in 0.9 i think)

Comment 43

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 338505 [details] [diff] [review]
second try for FF v2

I think we van apply this patch to the trunk, because the  “ is not used in the custom.properties anymore.

Comment 44

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 332061 [details] [diff] [review]
TB also includes editor

patch toegepast op l10n-central

Comment 45

[Tim Maks van den Broek [:mad_maks]]

(In reply to comment #44)
> (From update of attachment 332061 [details] [diff] [review])
> patch toegepast op l10n-central

but removed the quote symbols from custom.properties

Comment 46

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 338505 [details] [diff] [review]
second try for FF v2

patch applied on l10n-central/nl

Comment 47

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 332057 [details] [diff] [review]
second try for SM

patch applied on l10n-central/nl

Comment 48

[Tim Maks van den Broek [:mad_maks]]

Comment on attachment 338734 [details] [diff] [review]
second try for SB v3

patch applied on l10n-central/nl

Comment 49

[Axel Hecht [:Pike]]

Comment on attachment 338505 [details] [diff] [review]
second try for FF v2

a=me for 3.0.4, please land with a check-in comment referencing this bug and my approval, and use the fixed1.9.0.4 and verified1.9.0.4 keywords to track landing and testing.

Comment 50

[Tim Maks van den Broek [:mad_maks]]

patch applied on the cvs trunk

Comment 51

[u81239]

Only on trunk, or also on the 1.9.0.4 branch?

Comment 52

[Tim Maks van den Broek [:mad_maks]]

(In reply to comment #51)
> Only on trunk, or also on the 1.9.0.4 branch?

the cvs trunk is the branch for the 3.0.x builds