Skip to content

Problems with string handling. #63

Closed
jstressman opened this Issue Nov 21, 2011 · 8 comments

2 participants

@jstressman

While preparing and testing the Japanese translation, I noticed some problems with the ways strings are being handled. One might be just a bug, the other appears to be a lack of functionality.

For example, first let's look at

``` 'youHaveBeenKickedBy': 'You have been kicked from %s by %s',


which is passed 2 arguments by...

```                     actionLabel = $.i18n._('youHaveBeenKickedBy', [args.roomName, actorName]);

Now there are apparently 2 problems here.

First is that only the first argument is repeated, leaving us showing the room name for both the room name %s and the username %s.

http://i32.photobucket.com/albums/d12/phreadom/stringbugs2.png

This could just be a bug in the code that is setting the room name for the username...

I thought it might be something like this spot around line 2475... but I can't follow the code easily enough at a glance to figure it out.

``` var actorName = args.actor ? Strophe.getNodeFromJid(args.actor) : args.roomName,
actionLabel;


Second... and this appears to be an issue of lacking functionality... when preparing i18n translations, we __**really**__ should have the ability to reorder string variables.

For instance, the way sprintf handles this is by allowing you to specify the order of the variables to be used...

So for instance we could change

```     'youHaveBeenKickedBy': 'You have been kicked from %s by %s',

to

``` 'youHaveBeenKickedBy': '%2$s kicked you from %1$s',

to use the username first, and the roomname second, even though the roomname was given first in the original arguments.

Javascript allows this: http://www.diveintojavascript.com/projects/javascript-sprintf
Perl allows this: http://perldoc.perl.org/functions/sprintf.html
PHP allows this: http://php.net/manual/en/function.sprintf.php

etc. Is there some way to do this within your current framework that I'm just overlooking? (since the method I illustrate doesn't work)

Until we can get a fix for this issue, there are a few lines of the Japanese translation that will be reversed.

Japanese, like many other languages, is Subject-Object-Verb  ( http://en.wikipedia.org/wiki/Subject%E2%80%93object%E2%80%93verb ) while English is Subject-Verb-Object ( http://en.wikipedia.org/wiki/Subject%E2%80%93verb%E2%80%93object ).

And of course there are other orders as well. I'm sure you can see how it would be a great benefit to us to be able to reorder the string variables as needed for the different languages.

Once we get this sorted out we can add in the Japanese translation with the variables in the proper places. :)
@mweibel
Candy Chat member
mweibel commented Nov 21, 2011

Hi jstressman,

thanks for posting this. That's something we should have a look into.
We're using jquery-i18n for the translation and I currently don't see any way on how to correctly order those values.
I'll open a ticket for that.

Regarding the kicked issue: strange :) Seems to be a bug.

  • Michael
@mweibel
Candy Chat member
mweibel commented Nov 21, 2011

I just commited a fix to the i18n plugin and as soon as the author accepts it, we can fix your problem.

@mweibel
Candy Chat member
mweibel commented Nov 21, 2011

@jstressman: Regarding the other problem with "youHaveBeenKickedBy". Could you please post the xml element which sends the "kick"-message to the user?
The code for the "actorName" does use the roomName if there isn't any actor in the xml stanza and I think that's happening to you.
Maybe different servers send those presence changes in a different way and we'll need to fix that.

@jstressman

Is this the stanza you're referring to?

http://i32.photobucket.com/albums/d12/phreadom/kickstanza1.png

<body xmlns='http://jabber.org/protocol/httpbind' sid='#removed#' xmlns:stream='http://etherx.jabber.org/streams'>
  <presence xmlns='jabber:client' type='unavailable' to='phreadom@nnb.me/Candy' from='lounge@chat.nnb.me/phreadom'>
    <x xmlns='http://jabber.org/protocol/muc#user'>
      <item affiliation='none' nick='phreadom' role='none'>
        <reason>testing again</reason>
      </item>
      <status code='307'/>
    </x>
  </presence>
</body>
@mweibel
Candy Chat member
mweibel commented Nov 22, 2011

Exactly.
See XEP-0045 for reference:
http://xmpp.org/extensions/xep-0045.html#kick

But maybe we should, instead of just using the room name as the actor, don't display the "by actor" sentence if there isn't any actor.
This would require to add another sentence, I think.

@jstressman

hrm... maybe I need to try this on a more vanilla Candy install? :(

Trying it on both my local and remote servers kicking a user from within Candy itself still results in the same roomName twice problem.

When I tried it locally, this is all I see in my Prosody log:

mod_bosh        debug BOSH stanza received: <iq id='kick1' type='set' to='lounge@chat.nnb.phreadom.com
' from='largo@nnb.phreadom.com'>

bosh<number removed>        debug Received[c2s]: <iq id='kick1' type='set' to='lounge@chat.nnb.phreadom.com' from=
'largo@nnb.phreadom.com'>

when user largo kicks user demo1. The rest of the info is essentially the same as I already showed from the Web Developer log in Firefox.

@jstressman

Ah... the Prosody developers are helping enlighten me on this. Apparently the actor part of the stanza is left out at that level, so Candy is just reusing the roomName string.

They said that the actor part is only optional, so we can't count on it being there... so your suggestion about another sentence basically matches their suggestions.

@mweibel
Candy Chat member
mweibel commented Nov 22, 2011

Yeah, that was basicly my suggestion :D
I'll take a look if I can push a fix soon

@mweibel mweibel added a commit that closed this issue Nov 30, 2011
@mweibel mweibel Fixed #63: Fix youHaveBeenKicked/BannedBy to support kick/ban message…
…s without an actor

fixed #64: adding japanese translation

fixed accidental overwrite of chinese translation (raptorMessageBlocked again..)

fixed #56: Updating french translation, thanks to @ajoly
4aff6dd
@mweibel mweibel closed this in 4aff6dd Nov 30, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.