To reproduce:

a) Turn on Language and Content Translation modules.

b) Add a 2nd language to your site.

c) Configure Page nodes to be translatable.

d) Add a node. Translate it.

e) Go to the translations tab, such as node/1/translations. You'll see that it says <span class="status">Published</span> right there on the screen. Oops:
screen shot of translations page of node

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ericski’s picture

This should be considered a child issue under the larger meta issue of https://www.drupal.org/node/2297711

Not strictly a duplicate, so I'm not entirely sure on the etiquette here.

jhodgdon’s picture

OK, let's make it a child issue then. Thanks!

Gábor Hojtsy’s picture

Issue tags: +D8MI, +language-content
philipnorton42’s picture

This looked like a simple thing to sort out so I've had a bash at it. Not sure if this is the right approach, but it seems to work.

dawehner’s picture

Status: Active » Reviewed & tested by the community

This is fine for now, it would be great to add a followup for the todo already.

chx’s picture

Status: Reviewed & tested by the community » Needs work

Please don't do this. There's already a // @todo Add a theming function here. in HEAD -- please add a Twig template. We are trying to remove SafeMarkup::set calls instead of adding even more.

jhodgdon’s picture

Status: Needs work » Postponed

Postponing on the parent issue.

star-szr’s picture

Adding a very related issue.

vijaycs85’s picture

Michelle’s picture

I've been working with chx on IRC to fix this using the new renderInline. This is the code he came up with in this patch. When I tested it, this is what happened:

1st time reloading the page after clearing the cache, there was no error but no visual effect. The markup still showed as it had with the original code.

Subsequent reloadings without clearing the cache resulted in this error:

Uncaught PHP Exception Twig_Error_Loader: "Unable to find template "{{ status }}" (looked into: /var/www/html/d8.dev)." at /var/www/html/d8.dev/core/vendor/twig/twig/lib/Twig/Loader/Filesystem.php line 202

Michelle’s picture

Status: Active » Needs review

Oh! I forgot to change the status. First time doing a patch, sorry. :)

vijaycs85’s picture

Minor:

+++ b/core/modules/content_translation/content_translation.pages.inc
@@ -97,8 +97,15 @@ function content_translation_overview(EntityInterface $entity) {
+          $status = $twig->renderInline('<span class="status">{{ status }}</span>', $args);
+        }
+        else {
+          $status = $twig->renderInline('<span class="status">{{ status }}</span><span class="marker"> {{"outdated"|t}} </span>', $args);
+        }

can be improved by

         $args  = array('status' => $status);
         $message = '<span class="status">{{ status }}</span>';
         if (!empty($translation['outdated'])) {
           $message .= '<span class="marker"> {{"outdated"|t}} </span>';
         }
         $status = $twig->renderInline($message, $args);
chx’s picture

Please be careful not doing that. renderInline is like t() and should not have variables in it.

dawehner’s picture

Didn't we wanted to not use the service directly but rather always use the render element?

Gábor Hojtsy’s picture

@dawehner/@chx: which way is preferred is not documented at https://www.drupal.org/node/2311123, they are explained as equal alternates. As for which one is translation supported, if core supports both, we need to make a best effort basis to support both. See #2315329: Support for Drupal 8 inline twig translatable. The variable replacement use like @vijaycs85 did would never work for translation of course. The more special we get with these APIs, the harder it gets to explain to people what limitations they should consider for translatability :/

dawehner’s picture

@dawehner/@chx: which way is preferred is not documented at https://www.drupal.org/node/2311123, they are explained as equal alternates. As for which one is translation supported, if core supports both, we need to make a best effort basis to support both. See #2315329: Support for Drupal 8 inline twig translatable. The variable replacement use like @vijaycs85 did would never work for translation of course. The more special we get with these APIs, the harder it gets to explain to people what limitations they should consider for translatability :/

Updated the change record. The API documentation says that the render element should be used:

   * Warning: You should use the render element 'inline_template' together with
   * the #template attribute instead of this method directly.
   * On top of that you have to ensure that the template string is not dynamic
   * but just an ordinary static php string, because there may be installations
   * using read-only PHPStorage that want to generate all possible twig
   * templates as part of a build step. So it is important that an automated
   * script can find the templates and extract them. This is only possible if
   * the template is a regular string.
star-szr’s picture

Status: Needs review » Needs work

Thanks for working on this @Michelle!

Once we get the above sorted out, just a very minor note that Twig coding standards (http://twig.sensiolabs.org/doc/coding_standards.html) that we are using (https://www.drupal.org/node/1823416) mean that the curly braces need spaces inside of them. And we usually use single quotes.

So {{ 'outdated'|t }} instead of {{"outdated"|t}}.

Michelle’s picture

Just make sure chx gets credit for the code. We started out with the idea that I would do this but then he ended up writing the code and I just tested it and made the patch. :)

Thanks for the coding standards link. I need to go over that and learn all the new stuff.

Michelle’s picture

Issue tags: +TCDrupal 2014

I am attempting to figure out why this isn't working at TCDC.

jerdavis’s picture

I re-rolled this using render arrays as recommended here https://www.drupal.org/node/2311123, and as implemented in a similar fix by Cottser here https://www.drupal.org/node/2305831#comment-9033769

This fixes the issue.

It appears the base problem with calling $twig->renderInline() in this context is that the rendered HTML is then added to the render array for the table a bit further down in the code. That html string is not marked safe, and thus gets escaped later on when the table is rendered. (my assumption anyway). By providing a render array instead, it all renders out as expected.

jerdavis’s picture

Status: Needs work » Needs review
joelpittet’s picture

Status: Needs review » Needs work

Would be so nice if that code could be in a template but we've not had such luck with that yet. This looks correct to me. Just a couple of things I noticed:

  1. +++ b/core/modules/content_translation/content_translation.pages.inc
    @@ -97,9 +97,26 @@ function content_translation_overview(EntityInterface $entity) {
    -        $status = '<span class="status">' . $status . '</span>' . (!empty($translation['outdated']) ? ' <span class="marker">' . t('outdated') . '</span>' : '');
    

    Is it supposed to be classs 'status' or class 'marker'?

  2. +++ b/core/modules/content_translation/content_translation.pages.inc
    @@ -97,9 +97,26 @@ function content_translation_overview(EntityInterface $entity) {
    +              '#template' => '<span class="status">{{ status }}</span><span class="marker">{{ \'outdated\'|t }}</span>',
    

    You could use double quotes on the inside or outside to avoid the quote escaping.
    @see https://www.drupal.org/coding-standards#quotes

Michelle’s picture

Status: Needs work » Needs review

@#22:

#1 - It's class "status" around the status and class "marker" around "outdated". I'm not seeing any issue here. The patch is putting the same markup as the code it is replacing.

#2 - In the patch I posted, chx/I used double quotes around "outdated" and it was called out in comment #17. I'm assuming that's why it's escaped in jerdavis's patch.

Since #1 seems fine and #2 is fixing per a previous review, I'm setting this back to needs review.

joelpittet’s picture

re #23
1) Oh I misread that, thank you.
2) @Cottser, maybe you want to revisit this or ping me on irc?

star-szr’s picture

Status: Needs review » Closed (duplicate)

Oops. Looks like this is now officially a duplicate of #2250841: Adding a inline template for content translation status, sorry and thank you!

jibran’s picture

Issue tags: +SafeMarkup