Opened on 09/04/2018 at 03:40:34 PM

Closed on 10/26/2018 at 02:36:49 PM

#6928 closed change (fixed)

[CMS] Translation strings without default values

Reported by: kvas Assignee:
Priority: P3 Milestone:
Module: Sitescripts Keywords: goodfirstbug
Cc: juliandoucette Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://gitlab.com/eyeo/websites/cms/merge_requests/2/

Description (last modified by kvas)

Background

•julian> vasily: I'd like to share a string across pages that includes a link. However, it seems the href is usually taken out of strings in locale files and added back somehow. Do you have any ideas about this?
5:27 PM <•julian> (I'm talking about sharing a string using get_string, not an include, because I don't want the string to be re-translated per page)
5:27 PM <vasily> julian: not out of the top of my head, but I can look into this
6:35 PM <vasily> julian: the attributes of the tags inside of translation strings (and the values of those attributes) are taken from the default translation and then inserted into any non-default translation (the matching is done based on the tag name and, if there are several instances of the same tag in the string, on the order of the tags).
6:36 PM <vasily> non-default translations are not allowed to have their own attributes, however, the values of attributes in the default translation can have further translation strings inside of them -- this allows language-specific customization
6:36 PM <vasily> julian: this is my interpretation of the code...
6:37 PM <vasily> julian: I can point you to the code, if you'd like, but it's kind of hairy...
6:37 PM <vasily> julian: does this help?
7:01 PM <•julian> vasily: So can you change the CMS so that I can share a string with a link in it across pages?
9:35 PM <vasily> julian: so the problem with the current approach is that the translation strings from localization files are not allowed to have attributes, and as a result you can't put a URL in it, right?
9:35 PM <vasily> julian: what we need is something like "get the translation string without a default value and take the attribute values from the translation string" -- do i get it right?
9:37 PM <vasily> julian: seems like this would be possible. it would require changes in that hairy part of the code that i mentioned before, but uh oh, i guess we gotta unhairify it one day
12:58 AM <•julian> > 
so the problem with the current approach is that the translation strings from localization files are not allowed to have attributes, and as a result you can't put a URL in it, right?
12:58 AM <•julian> Right.
12:58 AM <•julian> > 
what we need is something like "get the translation string without a default value and take the attribute values from the translation string" -- do i get it right?
12:58 AM <•julian> Right.
12:58 AM <•julian> > 
seems like this would be possible. it would require changes in that hairy part of the code that i mentioned before, but uh oh, i guess we gotta unhairify it one day
12:58 AM <•julian> Sooner than later I hope :D

What to change

Load default text from default language translation file when it's not specified in the translation string. If the translation file doesn't have translation for the string id, produce an error (this is current behavior when there's no default).

Explanation and justification

The general translation string syntax is as follows: {{$id[$comment] $default-text}}. Comment is optional and default text can be omitted if another string with the same id is present earlier in the same page. Translation strings are replaced by generated translation that depends on the language:

  • For the default language default text is used.
  • For non-default languages:
    • Tag attributes and fixed strings are extracted from the default text,
    • Translated text is retrieved based on id and locale (if it's missing, default text is used),
    • Tag attributes and fixed strings are inserted into the translated text.

Since the default text can only live in the page and the text for non-default languages is not allowed to have tag attributes, attribute values can't be shared between pages. This is the problem that this ticket aims to solve.

The easiest solution seems to be to make it possible to load default text from the translation file for the default language. For example, if we have a string `{{foo[bar]}} in the page, we could provide a default text for it if the translation file contains:

"foo": {
  "message": "foo default text",
  ...
}

Attachments (0)

Change History (14)

comment:1 Changed on 09/04/2018 at 03:40:51 PM by kvas

  • Owner set to kvas

comment:2 Changed on 09/11/2018 at 01:47:47 PM by kvas

  • Description modified (diff)

comment:3 Changed on 09/11/2018 at 03:19:09 PM by kvas

  • Cc juliandoucette added
  • Description modified (diff)
  • Keywords goodfirstbug added
  • Priority changed from Unknown to P3

comment:4 Changed on 09/17/2018 at 11:50:23 AM by juliandoucette

How would this effect our future plans for TMS agnostic translations?

comment:5 Changed on 09/17/2018 at 12:37:08 PM by kvas

This doesn't really change much in terms of dependence on a particular TMS (or on having a TMS in general). The only difference here is that it will be possible to place default translation text into the translation file for default language instead of the page itself. This doesn't affect the syntax of the translation strings or the way they are sourced (from the point of TMS integration it doesn't matter whether the default translation text is coming from the page or from the default locale JSON file). Perhaps you see something I don't see?

comment:6 Changed on 09/17/2018 at 02:09:21 PM by juliandoucette

Perhaps you see something I don't see?

IIUC our translation sync script compares the original string to the defaultlocale translation to determine if it has been changed. And if the original string is the defaultlocale string then that won't work?

comment:7 Changed on 09/17/2018 at 03:42:24 PM by kvas

Right now, as far as I can see, the default translation in, say, web.adblockplus.org is mostly empty and the CMS doesn't use it for normal translation strings (it takes the default translations from the pages).

We did have a plan to compare what's in the pages with what's in the translation files but my impression (confirmed with Tudor) is that we have rolled back on this after deciding to do one XTM project per repository and not one project per issue. Now I think it's unlikely that we'll store a local copy of the extracted/uploaded strings because in the situation when an XTM project might be shared between several developers (because project per repo), the local copy might not match the server data anyway and it's safer to assume that the files might have been changed on the server and work under this assumption. Thanks for flagging this anyway though -- this is something to pay attention to.

One idea for future-proofing this is to use a different key in the JSON file, for example default instead of message, which I was planning to use. At the moment there's no extra cost to doing this and it would allow tracking of changes to default text (if the value of default key is different from message, then we know that the translation string has changed). What do you think?

comment:8 follow-up: Changed on 09/19/2018 at 11:51:49 AM by juliandoucette

What do you think?

SGTM

---

PS: I don't like the locale file keys "message" and "description". Feel free to change them if you find a painless way to do so.

comment:9 in reply to: ↑ 8 Changed on 09/19/2018 at 02:28:23 PM by kvas

  • Description modified (diff)
  • Ready set

Replying to juliandoucette:

What do you think?

SGTM

Thanks! I have updated the ticket.

---

PS: I don't like the locale file keys "message" and "description". Feel free to change them if you find a painless way to do so.

I think these key names are coming from localization files of Mozilla/Chrome extensions (as far as I can see, maybe that's also not the original origin). We have also communicated this to XTM already, so I think there would be a bit of a pain in changing them.

comment:10 Changed on 10/04/2018 at 09:34:39 AM by kvas

  • Owner kvas deleted

comment:11 Changed on 10/25/2018 at 06:25:07 PM by kvas

  • Description modified (diff)
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

After more thinking I have decided to just use the "message" key in the default language file. This is simple, and also fixes #148 out of the box.

comment:12 Changed on 10/26/2018 at 02:36:06 PM by kvas

  • Review URL(s) modified (diff)

comment:13 Changed on 10/26/2018 at 02:36:20 PM by abpbot

comment:14 Changed on 10/26/2018 at 02:36:49 PM by kvas

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.