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:
Comment | File | Size | Author |
---|---|---|---|
#20 | html_tags_raw_shown-2305799-20.patch | 1.57 KB | jerdavis |
#10 | html_tags_raw_shown-2305799-10.patch | 1.29 KB | Michelle |
#4 | html_tags_raw_shown-2305799-4.patch | 1.3 KB | philipnorton42 |
raw-html.png | 23.54 KB | jhodgdon |
Comments
Comment #1
ericski CreditAttribution: ericski commentedThis 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.
Comment #2
jhodgdonOK, let's make it a child issue then. Thanks!
Comment #3
Gábor HojtsyComment #4
philipnorton42 CreditAttribution: philipnorton42 commentedThis 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.
Comment #5
dawehnerThis is fine for now, it would be great to add a followup for the todo already.
Comment #6
chx CreditAttribution: chx commentedPlease 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.
Comment #7
jhodgdonPostponing on the parent issue.
Comment #8
star-szrAdding a very related issue.
Comment #9
vijaycs85#2289999: Add an easy way to create HTML on the fly without having to create a theme function / template is in
Comment #10
MichelleI'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
Comment #11
MichelleOh! I forgot to change the status. First time doing a patch, sorry. :)
Comment #12
vijaycs85Minor:
can be improved by
Comment #13
chx CreditAttribution: chx commentedPlease be careful not doing that. renderInline is like t() and should not have variables in it.
Comment #14
dawehnerDidn't we wanted to not use the service directly but rather always use the render element?
Comment #15
Gábor Hojtsy@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 :/
Comment #16
dawehnerUpdated the change record. The API documentation says that the render element should be used:
Comment #17
star-szrThanks 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}}
.Comment #18
MichelleJust 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.
Comment #19
MichelleI am attempting to figure out why this isn't working at TCDC.
Comment #20
jerdavisI 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.
Comment #21
jerdavisComment #22
joelpittetWould 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:
Is it supposed to be classs 'status' or class 'marker'?
You could use double quotes on the inside or outside to avoid the quote escaping.
@see https://www.drupal.org/coding-standards#quotes
Comment #23
Michelle@#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.
Comment #24
joelpittetre #23
1) Oh I misread that, thank you.
2) @Cottser, maybe you want to revisit this or ping me on irc?
Comment #25
star-szrOops. Looks like this is now officially a duplicate of #2250841: Adding a inline template for content translation status, sorry and thank you!
Comment #26
jibran