rubenv / angular-gettext-tools

Tools for extracting/compiling angular-gettext strings.
http://angular-gettext.rocketeer.be/
MIT License
39 stars 130 forks source link

Breaking change in 2.1.11 #125

Open paulpdaniels opened 8 years ago

paulpdaniels commented 8 years ago

We use json scaffolds in our code configuration and the inclusion of the character code replacement during compilation is causing breaks in our strings because after compilation the string keys are no longer matching.

Consider:

{
  "title" : "Let’s create a profile"
}

After translation we have a .po file that has:


msgid "Let’s create your account"
msgstr "Blah blah blah"

However after compilation we end up with:


{
  "Let’s create a profile" : "Blah blah blah"
}

Not sure if this will happen in all cases but when used as:


<label for="{{component.id}}" ng-bind-html="component.title | translate"></label>

The translation fails because the strings won't match up.

This likely should have been more than a patch version bump, but at the very least we need some way to configure this behavior so we can switch it off. For now we are simply pegging our version to 2.1.10 until this is resolved.

rubenv commented 8 years ago

Ping @rubenswieringa

rubenswieringa commented 8 years ago

Sorry it took me a while to reply; just got back from holidays. My PR (#116/#124) didn't take ng-bind-html into account and that seems to be the problem (the translate filter itself seems to be working alright though). Looking into possible solutions now.

rubenswieringa commented 8 years ago

@paulpdaniels are you having trouble in all browsers or just some? And does your project use jquery?

The below tests for angular-gettext pass in karma:unit (I'm having trouble getting karma:unit_nojquery to work and not a lot of time to look into it any further just right now).

Also, I tried the ng-bind-html out in a personal project (Google Chrome 47) and didn't have any trouble there either.

describe("Stuff", function () {
    var catalog = null;
    var $rootScope = null;
    var $compile = null;

    beforeEach(module("gettext"));

    beforeEach(module(function ($sceProvider) {
        $sceProvider.enabled(false);
    }));

    beforeEach(inject(function ($injector, gettextCatalog ) {
        $rootScope = $injector.get("$rootScope");
        $compile = $injector.get("$compile");
        catalog = gettextCatalog;
        catalog.setStrings("nl", {
            "Quote’": "Aanhalingsteken&rsquo;"
        });
    }));

    it("translate-filter works", function () {
        catalog.setCurrentLanguage("nl");
        var el = $compile("<h1>{{\"Quote&rsquo;\"|translate}}</h1>")($rootScope);
        $rootScope.$digest();
        assert.equal(el.text(), "Aanhalingsteken&rsquo;");
    });

    it("ng-bind-html works", function () {
        catalog.setCurrentLanguage("nl");
        var el = $compile("<h1 ng-bind-html=\"'Quote&rsquo;'|translate\"></h1>")($rootScope);
        $rootScope.$digest();
        assert.equal(el[0].innerHTML, "Aanhalingsteken’");
    });
});

@rubenv could you give the above a go in karma:unit_nojquery or give me some pointers as to why I'm getting Cannot POST /run (see below)?

Running "karma:unit_nojquery:run" (karma) task
Cannot POST /run
Warning: Task "karma:unit_nojquery:run" failed. Used --force, continuing.

If my changes do in fact pose a problem to ng-bind-html then we have 2 (not mutually exclusive) options:

rubenswieringa commented 8 years ago

...additionally we can see if we could modify the translate filter to convert html-entities to regular unicode characters

paulpdaniels commented 8 years ago

@rubenswieringa Sorry just getting back into testing this more in depth. It seems IE is the problem child as usual. I am digging a little further in but if you try it with some other characters like & and ¢ that seems to reproduce the problem more reliably than ’

paulpdaniels commented 8 years ago

A little more detail I have worked out:

Original Text Key Text Output: Chrome 48 Output: IE 9 - 11
'’' '’' 'translated' '’'
'’' '’' '’' '’'
'’' '’' '’' 'translated'
'’' '’' 'translated' 'translated'