1288443 - Improve fallback for label attributes in XUL

Status: NEW

(L20n :: JS Library, defect)

Description

[Staś Małolepszy :stas]

Many XUL elements don't have textContent.  Instead they use the label attribute to show content to the user.  In case of missing or broken translations it would be great if our XUL bindings could display the id of the string as the label.

Comment 1

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

I took a look at this and it seems that the we currently have no way from within DOM to know if the element resolved with errors or not.

The `l10n.formatEntities` just returns the best possible value, which as last resort returns an id as a value.

On IRC :stas suggested that we scan through the list of known elements for which the trait is the "default display value" and if the returned entity doesn't have the trait, it means it's an error.

I'd like to try to avoid that approach for two reasons:

1) It's very implicit
2) It seems performance unfriendly

Instead, I'd like to retrieve information from `l10n.formatEntities` on which entities did not resolve. That may go beyond just the use case for this bug.

In the future, we may want to mark them with some CSS (background-color: red), or flag otherwise as a localization bug.

My proposal would be to make `l10n.formatEntities` support levels of errors:

1) Entity resolved
2) Entity resolved in a fallback language
3) Entity partially resolved (undefined entity reference or variable)
4) Entity did not resolve

We currently would only care about (4), so if there's any performance cost of carrying this data around, we could just do the (4) now, but in the future I can see it being useful for debugging.

:stas, thoughts?

Comment 2

[Staś Małolepszy :stas]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #1)
> 
> I'd like to try to avoid that approach for two reasons:
> 
> 1) It's very implicit
> 2) It seems performance unfriendly

I've been thinking about #1 a lot.  On one hand I agree with you that implicit is not good.  OTOH, it's tempting to think that rather than:

    new-navigator-menuitem =
        [xul/label]      New Window
        [xul/accesskey]  N

the localizer could do:

    new-navigator-menuitem = New Window
        [xul/accesskey]  N

That's probably something to think about much later, though. As you suggest, let's avoid this approach completely.

> Instead, I'd like to retrieve information from `l10n.formatEntities` on
> which entities did not resolve. That may go beyond just the use case for
> this bug.

Sorry to block everything on bug 1316083 but what if we just overloaded formatEntities or entityFromContext in DOMLocalization?  We could then run this additional fallback logic there.

I'd prefer to not expose the errors to the outside world for now.  Yet another thing to support and maintain.  For the background-color use-case I think I'd suggest implementing a special DOMDebugLocalization subclass, perhaps not even in the library.

Comment 3

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

(In reply to Staś Małolepszy :stas from comment #2)
> Sorry to block everything on bug 1316083 but what if we just overloaded
> formatEntities or entityFromContext in DOMLocalization?  We could then run
> this additional fallback logic there.

When you hold a hammer... ;)

Sure, sounds good to me.

Comment 4

[Staś Małolepszy :stas]

Bug 1316083 and bug 1318086 both landed. It should now be possible to implement the special fallback behavior for XUL labels in DOMLocalization.entityFromContext.