Whitelist <button> in translations

RESOLVED INVALID

Status

RESOLVED INVALID
5 years ago
4 years ago

People

(Reporter: mgol, Unassigned)

Tracking

Details

Attachments

(1 attachment)

Created attachment 831940 [details] [diff] [review]
whitelist-button.patch

We have a few of places in our app where the translated div contains a button that needs to be translated as well but which shouldn't be translated separately as it's just a part of the sentence.

I think <button> should be allowed in translations similarly to <a> being available.

Patch attached.
Attachment #831940 - Flags: review?(stas)
The default whitelist of HTML elements only includes elements which define text-level semantics.  The full list is here:

    http://www.w3.org/html/wg/drafts/html/master/text-level-semantics.html

I think we should stick to that, as it seems to be the largest intersection of 'useful' and 'safe'.

Were we to allow buttons in translations by default, malicious localizers would be able to add them anywhere, creating an impression of broken UI or even hijacking event listeners if they're not attached by ids.

However, there is a simple way of allowing buttons (or any other blacklisted HTML elements) in any given translation:  add the <button> element to the source HTML, like so:

  <div data-l10n-id="foo">
    <button id="submit></button>
    <button id="cancel"></button>
  </div>

Then, in the translation, it will be possible to use it as well:

  <foo """
    <button>Submit</button> <em>or</em> <button>cancel</button>.
  """>

This has the desired effect, and at the same time allows you, as the developer, to control which translations can have button elements.

Does this work for you?  If so, can you please close this bug as wontfix?
(In reply to Staś Małolepszy :stas from comment #1)
> The default whitelist of HTML elements only includes elements which define
> text-level semantics.  The full list is here:

Ah, sorry, I thought it's the list of tags allowed to be overlayed and it's the list of tags that can be *inserted* by the translator, right?

> However, there is a simple way of allowing buttons (or any other blacklisted
> HTML elements) in any given translation:  add the <button> element to the
> source HTML, like so:
> 
>   <div data-l10n-id="foo">
>     <button id="submit></button>
>     <button id="cancel"></button>
>   </div>
> 
> Then, in the translation, it will be possible to use it as well:
> 
>   <foo """
>     <button>Submit</button> <em>or</em> <button>cancel</button>.
>   """>
> 
> This has the desired effect, and at the same time allows you, as the
> developer, to control which translations can have button elements.
> 
> Does this work for you?  If so, can you please close this bug as wontfix?

I tried that first and it doesn't work. I'm pretty sure it used to work but when I upgraded to the newest l20n.js it stopped overlaying the button and instead it just replaces it with the text put in the translation.
(Reporter)

Updated

5 years ago
Flags: needinfo?(stas)
To be more precise, my HTML is as follows:

    <li data-l20n="CookiePolicy">
        <button class="cookie-policy-link"
                cbn-launch-dialog="cbn-dialog-flatpage"
                cbn-flatpage-name="cookies"></button>
    </li>

and my translation file (English version) has this entry:

    <CookiePolicy "Polona uses cookies. More information <button>here</button>.">
Ah, sorry, I've checked it now and it works. :/ Sorry for the fuss, maybe code in some other place caused a chain of errors that lead to this...

I'll try to be more careful with makings sure the bug really exists in the future.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → INVALID
(Reporter)

Updated

5 years ago
Flags: needinfo?(stas)
Hey, no problem, I'm glad that it works :)  Always better to file a bug which turns out to not be a bug, than the other way around!
(In reply to Michał Gołębiowski from comment #2)

> Ah, sorry, I thought it's the list of tags allowed to be overlayed and it's
> the list of tags that can be *inserted* by the translator, right?

Yes, this is correct.
Attachment #831940 - Flags: review?(stas)
You need to log in before you can comment on or make changes to this bug.