1737439 - Users of Fluent should escape the < character in data-l10n-args when applying it to the text content of a node

Status: NEW

(Toolkit :: Performance Monitoring, defect)

Description

[Denis Lisov]

Given the following test data URL
data:text/html,<title>Generic<Some, Type, Parameters> description</title>
about:processes displays only "Generic description" as a title, omitting the generic parameters in < .. >

Comment 1

[Denis Lisov]

Sorry, missed escaping for Bugzilla
data:text/html,<title>Generic&lt;Some, Type, Parameters&gt; description</title>

Comment 2

[Chris Peterson [:cpeterson]]

Thanks for reporting this bug, Denis. I can reproduce it in Firefox 93, 94, and 95. The "Generic description" process is listed under about:processes page's "Shared Web Process" section.

@ Florian, does this look like a bug in the about:processes page or in the L10n code that processes data-l10n-args?

I inspected the about:processes page's HTML and see that the "Generic description" node's data-l10n-args attribute includes the expected <Some, Type, Parameters> substring that is not shown on the page:

<td data-l10n-id="about-processes-tab-name" data-l10n-args="{"name":"Generic<Some, Type, Parameters> description"}">Tab: Generic description</td>

Should the code that dynamically sets data-l10n-args first escape the angle brackets < >? Or is the L10n code stripping the <Some, Type, Parameters> substring when it shouldn't?

Comment 3

[Florian Quèze [:florian]]

This was already broken with the patch from bug 1652785 that introduced showing page titles in about:processes, which landed in Firefox 82.

Comment 4

[Florian Quèze [:florian]]

(In reply to Chris Peterson [:cpeterson] from comment #2)

Should the code that dynamically sets data-l10n-args first escape the angle brackets < >? Or is the L10n code stripping the <Some, Type, Parameters> substring when it shouldn't?

ni Zibi for this question.

Comment 5

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] from comment #4)

(In reply to Chris Peterson [:cpeterson] from comment #2)

Should the code that dynamically sets data-l10n-args first escape the angle brackets < >? Or is the L10n code stripping the <Some, Type, Parameters> substring when it shouldn't?

Thinking about this some more, I think it's a bug of the l10n code. The JS code calling document.l10n.setAttributes doesn't know if the data will be displayed as text content or as an attribute (that depends on how the ftl file uses the parameters, and the same parameter could even be used in both a text content of a node and an attribute).

For example if an ftl file includes a string like this:

string-name = Prefix: { $param }
    .attribute = Prefix: { $param }

The < character needs to be escaped for setting the content of the DOM node, but not when setting the attribute.

Looking at the code a bit, the code at https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/dom/l10n/L10nOverlays.cpp#492 checks if the translated string contains the < character, and calls SetTextContent if it doesn't, and will parse the markup otherwise.
I think this code needs to treat differently a < character coming from the ftl file, and a < character coming from the data-l10n-args attribute.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

I think this code needs to treat differently a < character coming from the ftl file, and a < character coming from the data-l10n-args attribute.

I don't think I agree with that. Args can provide HTML fragment as well:

let p = document.getElementById("mypar");
document.l10n.setAttributes(p, "my-key", {
  label: "This is <b>a label</b>"
});

my-key = Your label is: { $label }.

should produce:

<p id="mypar" data-l10n-id="my-key" data-l10n-args="{'label': 'This is <'b>a label</b>'}">
  Your label is: This is <b>a label</b>.
</p>

Escaping text level elements would make it impossible.

My read is that there are cases where you want those to be escaped, and in those cases the consumer should handle it when building the args list.

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

:eemeli - what do you think?

Comment 9

[Eemeli Aro [:eemeli]]

I'm with Zibi here. It's pretty much established that document.l10n will allow a subset of top-level tags to be rendered as elements in its output and filters out others. Changing this would be a breaking change. To see this, here's a test URL:

data:text/html,<title>Generic&lt;i&gt;&lt;b&gt;Some, Type, Parameters&lt;/b&gt&lt;/i&gt; description</title>

Note how document.title ends up as the string "Generic<i><b>Some, Type, Parameters</b></i> description", while the listing in about:processes renders it as "Generic Some, Type, Parameters description", i.e. rendering the outer <i> but dropping the <b>.

I think the right fix here is to escape the document.title before setting it as the label value.

Effecting the same result by a localization API change by adding some sort of a toggle for treating input as explicitly raw text would probably end up being quite a bit clumsier.

Comment 10

[Frederik Braun [:freddy]]

Having established that this is in the responsibility of the user of Fluent (and not within fluent itself), it seems the solution is then to pass an escaped document.title into the fluent string.
AFAIU, Fluent assigns to .innerHTML, so E.g., replacing all > and < in document.title would suffice.

That also means this bug should move into about:processes bugs and out of the Fluent components.

Comment 11

[Oriol Brufau [:Oriol]]

(In reply to Frederik Braun [:freddy] from comment #10)

AFAIU, Fluent assigns to .innerHTML, so E.g., replacing all > and < in document.title would suffice.

It would not suffice, you also need to escape & as &, e.g. data:text/html,<title>Some currencies; like ruble&amp;euro; will devaluate</title> should not appear as

Tab: Some currencies; like ruble€ will devaluate