1426835 - [FTL] HTML markup missing when localizing Test Pilot Website

Status: RESOLVED FIXED

(Webtools Graveyard :: Pontoon, defect, P1)

Description

[Selim Şumlu]

Attachment: Screenshot-2017-12-22 Test Pilot Website · Turkish (tr).png

Pontoon has been showing rendered versions HTML-coded strings for the last few days. (See the screenshot for an example.)

It's not possible to see and place the corect HTML tags unless you use the "copy" button first.

Comment 1

[Francesco Lodolo [:flod]]

Ouch, that's bad. 

Layout also is broken on the left column
https://pontoon.mozilla.org/tr/test-pilot-website/experiments.ftl/?string=159482
https://pontoon.mozilla.org/tr/test-pilot-website/experiments.ftl/?search=love+multi&string=159539

I assume this is a regression from bug 1392255.

Comment 2

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #1)
> I assume this is a regression from bug 1392255.

Actually, bug 1382348. mozilla.org still works correctly, so it's FTL only
https://pontoon.mozilla.org/tr/mozillaorg/all-resources/?string=7867

And I notice that HTML fragments are placeables?

Comment 3

[Matjaz Horvat [:mathjazz]]

Thanks for reporting Selim!

The broken layout is likely a regression from bug 1392255. We've added a bunch of changes after it landed, so I'd like to provide a fix instead of reverting. Let me dig into it.

As for the HTML markup in FTL source strings, we don't support that yet:
https://bugzilla.mozilla.org/show_bug.cgi?id=1382348#c2

We should add it once we start using DOM Overlays in FTL (should happen pretty soon).

Comment 4

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/790

Comment 5

[Matjaz Horvat [:mathjazz]]

Confirmed this is a regression from bug 1392255 and provided a fix.

I also realized we might be talking about different thing regarding the HTML markup. It's not even visible ATM for FTL strings and this patch fixes it. This is much more serious than marking it up like placeables, which is coming later. Raising priority to P1.

This is deployed to stage for testing. I'd appreciate if one of you could have a look:
https://mozilla-pontoon-staging.herokuapp.com/tr/test-pilot-website/experiments.ftl/?string=159482

I also checked some other FTL files and so far it seems to be working ok.

Comment 6

[Francesco Lodolo [:flod]]

It seems to work correctly on stage.

Not sure I understand if the code covers all cases, i.e. if there could be HTML code when markPlaceables is false. When is markPlaceables false in serializePlaceables?

Comment 7

[Selim Şumlu]

Yes, it seems fine on stage.

What does the "FTL" button supposed to do? For me, clicking on it places a "minvidIntroduction = " text on the translation field and nothing else. I'm not sure if it's related to this bug though.

Comment 8

[Francesco Lodolo [:flod]]

(In reply to Selim Şumlu from comment #7)
> Yes, it seems fine on stage.
> 
> What does the "FTL" button supposed to do? For me, clicking on it places a
> "minvidIntroduction = " text on the translation field and nothing else. I'm
> not sure if it's related to this bug though.

Switch to advanced/source mode
https://mozilla-l10n.github.io/localizer-documentation/tools/pontoon/ui.html#ftl-fluent-files

Comment 9

[Matjaz Horvat [:mathjazz]]

(In reply to Francesco Lodolo [:flod] from comment #6)
> It seems to work correctly on stage.
> 
> Not sure I understand if the code covers all cases, i.e. if there could be
> HTML code when markPlaceables is false. When is markPlaceables false in
> serializePlaceables?

We could have HTML code when markPlaceables is false, but that's either in rich FTL editor (we want to show HTML) or places like String list and History tab, where HTML is escaped in translate.js.

For that very reason I added a TODO to make the code more robust and easier to understand.

Comment 10

[[github robot]]

Commit pushed to master at https://github.com/mozilla/pontoon

https://github.com/mozilla/pontoon/commit/e2b17c08c2130940edff6d36e4f89d1fa2f01fe4
Fix bug 1426835: Do not render HTML in FTL strings (#790)