[email] L10nError: "message-header-state-none" not found in en-US

RESOLVED FIXED in Firefox OS v2.2

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jrburke, Assigned: jrburke)

Tracking

unspecified
2.2 S6 (20feb)
x86
Mac OS X

Firefox Tracking Flags

(b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
This is a non-fatal bug, only in master.

In the email app, I now see L10nErrors like this in the logcat when viewing the message list:

WAR: L10nError: "message-header-state-none" not found in en-US in app://email.gaiamobile.org/index.html

message-header-state-none does exist, but it looks like this in the properties file:

message-header-state-none.ariaLabel=


If I put any value after the = sign, it does not complain.

Defined here:

https://github.com/mozilla-b2g/gaia/blob/bbc853ff4d1bffad41d19bb47e066e3ba21c6709/apps/email/locales/email.en-US.properties#L392

This is used for code that setting the accessibility setting on a node to show if it is either a message that is "sending", "in error state", or "no state". The empty string was a way to create an empty ariaLabel for that "no state" case:

https://github.com/mozilla-b2g/gaia/blob/bbc853ff4d1bffad41d19bb47e066e3ba21c6709/apps/email/js/cards/message_list.js#L1403

Asking :gandalf for some guidance on the best practice for this situation.

Should l10n.js be accepting of keys that exist but just have no value, so maybe a `if (entry === undefined && !locale.entries.hasOwnProperty(id))` check in l10n.js?

Or should we use a different technique in the email code to reflect a case where we do not want the accessibility tools to read anything for the status of the node in the "no state" case?
Flags: needinfo?(gandalf)
My guess is that we use falsy check somewhere in the build system and ignore when the value is falsy (instead of strict checking).

My take is that we should allow for empty values. If :stas agrees I'll take this bug.
Flags: needinfo?(gandalf) → needinfo?(stas)
The trouble with the properties format is that there's no distinction between no value (undefined) and an empty string value.  Besides, our parser doesn't accept either:

  https://github.com/l20n/l20n.js/blob/827bc11f4724378c24a6cd8dd63043f82378810b/lib/l20n/format/properties/parser.js#L17

This should explain why L10n.js thinks the message-header-state-none string is missing.

For this particular bug, I would suggest to change the logic in the Email app and remove the aria-label attribute if sendState is none.  The spec uses the language of "empty or undefined" so it should be the same as setting an empty attribute aria-label="".

  http://www.w3.org/TR/wai-aria/complete#namecalculation 5.2.7.3. 

Zibi, I filed bug 1132038 to continue the discussion about empty/no values.
Flags: needinfo?(stas)
Created attachment 8563111 [details] [review]
[gaia] jrburke:bug1130647-email-l10n-missing-prop > mozilla-b2g:master
(Assignee)

Comment 4

4 years ago
Comment on attachment 8563111 [details] [review]
[gaia] jrburke:bug1130647-email-l10n-missing-prop > mozilla-b2g:master

As suggested in comment #2, just remove the l10n attribute if there is no sendState.
Attachment #8563111 - Flags: review?(bugmail)
(Assignee)

Updated

4 years ago
Assignee: nobody → jrburke
Target Milestone: --- → 2.2 S6 (20feb)
Comment on attachment 8563111 [details] [review]
[gaia] jrburke:bug1130647-email-l10n-missing-prop > mozilla-b2g:master

arrrr+
Attachment #8563111 - Flags: review?(bugmail) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed

Updated

4 years ago
Keywords: checkin-needed

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

4 years ago
Comment on attachment 8563111 [details] [review]
[gaia] jrburke:bug1130647-email-l10n-missing-prop > mozilla-b2g:master

[Approval Request Comment]

[Bug caused by] (feature/regressing bug #):
Bug 1068699, added an empty l10n string, but the warning code in l10n.js does not like it. Since bug 1068699 is on 2.2, then this fix should be too.

[User impact] if declined:
No user impact, but cleans up logcat noise, which helps 

[Testing completed]:
On device, unit test updated.

[Risk to taking this patch] (and alternatives if risky):
Very low risk. Removing the data-l10n-id is a common way to remove a node from the l10n update machinery.

[String changes made]:
An l10n property string was removed, so no harm if it was translated already, it will just not be used by the code. The string was introduced in bug 1068699.
Attachment #8563111 - Flags: approval-gaia-v2.2?

Updated

4 years ago
Attachment #8563111 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/194df71e383ba546c4ebfeb0bb624f60e4263a7d
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
Hey guys, just an update. I landed bug 1132038 so now empty values will work :)
You need to log in before you can comment on or make changes to this bug.