1025556 - Can't see the "View" button after downloading an attached picture when it's not localized

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Théo Chevalier]

The "View" button after downloading an attached picture is not displayed anymore in any locale.
If you touch the empty area where the button is supposed to be, you can still activate the button, but you can't see it.

My guess is that it's the same issue as on bug 1025554, but since the text is not displayed, we can't see the button.

The affected string:
message-attachment-view.textContent=View

Not adding late-l10n keyword yet, but if we have to remove ".textContent" from the entity name, it will definitely be a late-l10n bug.

Comment 1

[Francesco Lodolo [:flod]]

Same question of bug 1025554. This time the bug that changed the string on master is bug 1021946.

I see the button near the image on master.

Comment 2

[Théo Chevalier]

The string changed on master, indeed. However we should see the English string in other languages when there is no localization for this string.

Comment 3

[Francesco Lodolo [:flod]]

True that. Stas, any thought?

Comment 4

[Staś Małolepszy :stas]

foo.textContent is not supposed to work on master.  We turned that off in bug 994290.  See dependencies of that bug for work we did to stop using .textContent in Gaia.

Théo, are you seeing this bug on other branches than master as well?  If it's only master, why would this be a late-l10n bug change?

Comment 5

[Francesco Lodolo [:flod]]

Stas, the problem is that on nightly, if the string is missing from the locale (comment 2), nothing is displayed instead of showing the en-US label.

Comment 6

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

Flod: if another locale has 

entity.attr = Foo

we will not treat it as if "entity" does not exist (and take the en-US fallback).

We must actually move the entity.attr = Foo -> entity = Foo in locales.

Comment 7

[Francesco Lodolo [:flod]]

(In reply to Zibi Braniecki [:gandalf] from comment #6)
> we will not treat it as if "entity" does not exist (and take the en-US
> fallback).

I think that's a problem, and "entity" should be treated as a separate string from "entity.attribute" (that's also what compare-locales does).

Comment 8

[Axel Hecht [:Pike]]

Shouldn't "no value for entity" be a runtime error and fall back based on that?

Comment 9

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

(In reply to Axel Hecht [:Pike] from comment #8)
> Shouldn't "no value for entity" be a runtime error and fall back based on
> that?

No, there are a lot of entities that only have attributes, and it seems that gaia wants to be able to define a node with "hardcoded" value, and localizable attribute - say,

<button data-l10n-id="foo"><img/></button>

foo.ariaLabel = text

If you think we should forbid that, it'll be some work to transition away from using such code.

Comment 10

[Staś Małolepszy :stas]

(In reply to Zibi Braniecki [:gandalf] from comment #9)
> (In reply to Axel Hecht [:Pike] from comment #8)
> > Shouldn't "no value for entity" be a runtime error and fall back based on
> > that?
> 
> No, there are a lot of entities that only have attributes, and it seems that
> gaia wants to be able to define a node with "hardcoded" value, and
> localizable attribute - say,
> 
> <button data-l10n-id="foo"><img/></button>
> 
> foo.ariaLabel = text
> 
> If you think we should forbid that, it'll be some work to transition away
> from using such code.

This will not work when we land bug 994357.  The translation will need to explicitly sport the inner <img> element somewhere, or it will be removed from the localized node.  I don't think that we should try to guess where the localizer intended to put the translated context (before, after or around the inner element).

Note that currently, setTextContent in l10n.js uses a simple heuristic of inserting translation in place of the first child text node, and this is something that we wanted to clean up.

Comment 11

[James Burke [:jrburke]]

A proper solution for master and the new l10n.js behavior for `textContent` strings should still be tracked in this bug, but note that bug 911412 will switch to use an icon for that button instead of text. 

We still want the right localized text for accessibility reasons, but as far as display purposes, bug 911412 should help things to be more visually consistent.

Comment 12

[BMO Automation]

Firefox OS is not being worked on