Staś Małolepszy :stas Reporter
	Description • 8 years ago

I've seen this in bug 1288406.  Given the following translation:

    downloads-key =
        [xul/key]        { OS() ->
            [lin]    y
           *[other]  j
        }

and the XUL source:

    <key id="key_openDownloads" data-l10n-id="downloads-key" command="Tools:Downloads" modifiers="accel,shift"/>

the Ctrl+Shift+Y key shortcut is broken and doesn't trigger anything.  That said, I could see `key="y"` in the DOM inspector which suggests both OS() and the -> (the selection expression) worked fine.

    <key id="key_openDownloads" data-l10n-id="downloads-key" command="Tools:Downloads" modifiers="accel,shift" key="⁨y⁩"/>

This might be a nasty race condition.

	Zibi Braniecki [:zbraniecki][:gandalf]
	Comment 1 • 8 years ago

I looked into this and it seems that the OS() works on linux and mac as expected.

The only reason I could find for the difference between linux and not-linux is the modifiers attribute.

On linux it's modifiers="accel", on macos/win it is modifiers="accel,shift".

If we can add it to whitelisted attributes, I believe it will work.

	Staś Małolepszy :stas Reporter
	Comment 2 • 8 years ago

I'm glad that it works.  I filed this two months ago and don't remember the details.  Perhaps I made a mistake when testing or one of the m-c merges fixed this.  In any case thanks for looking into this in depth!

IIUC we have a choice of adding `modifiers` to the list of localizable XUL attributes vs continuing to use #ifdefs in the XUL code.  In both cases we could get rid of *-mac, *-win and *-unix entities.

Pike, what's you preference?

	Axel Hecht [:Pike]
	Comment 3 • 8 years ago

I'm confused, I don't see how the modifiers do anything for us here, 'cause they didn't change at all.

Also, looking at a limited experiment with devtools, it seems the the key element doesn't actually listed to DOM changes :-/

	Zibi Braniecki [:zbraniecki][:gandalf]
	Comment 4 • 8 years ago

> I'm confused, I don't see how the modifiers do anything for us here, 'cause they didn't change at all.

So,

We have this ifdef: http://hg.mozilla.org/projects/larch/file/tip/browser/base/content/browser-sets.inc#l223
and we have those two entities: http://hg.mozilla.org/projects/larch/file/tip/browser/locales/en-US/browser/menubar.ftl#l300

Now, thanks to OS() macro we can reduce it to:

  <key id="key_openDownloads" data-l10n-id="downloads-key" command="Tools:Downloads" modifiers="accel"/>


    downloads-key =
        [xul/key]        { OS() ->
            [lin]    y
           *[other]  j
        }

but the problem we face is that in the current code modifiers are different for linux and non-linux.

:stas is giving two solutions:

1) We can preserve the ifdef preprocessing instruction
2) We can add `modifiers` to allowed attributes and do:

    downloads-key =
        [xul/key]        { OS() ->
            [lin]    y
           *[other]  j
        }
        [xul/modifiers]  { OS() ->
            [lin]    accel,shift
           *[other]  accel
        }

What is your preference?

	Axel Hecht [:Pike]
	Comment 5 • 8 years ago

I'm torn on changing how we do modifiers, just because localizers usually get things like that wrong.

This bug is about the control keys working the way that l20n.js sets them. The strongest way to test that that's true is to have a language switch that introduces a different set of keybindings. Can we test that that works?

	Staś Małolepszy :stas Reporter
	Comment 6 • 8 years ago

That involves packaging, I wonder if we can avoid that. How about we create a new translation with different attribute values and setAttribute its id on the element?

	Axel Hecht [:Pike]
	Comment 7 • 8 years ago

Yeah, that'd be another code path that we can use to check this.

	Staś Małolepszy :stas Reporter
	Comment 8 • 8 years ago

I was able to verify that the changes to the <key> element's attributes take effect only if the parent <keyset> is explicitly re-added to the document.

    https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/key#Related

I tested with:

    const key = document.getElementById('key_viewSource');
    const keyset = key.parentNode;
    keyset.parentNode.appendChild(keyset);

My further testing suggests that our current implementation might be racy.  If I delay l20n's startup logic by a few seconds, after it runs some command keys work as expected (Accel+T) while others break (Accel+U).  Actually most of them break.  In fact, I'm not completely sure why this works correctly on larch right now.  Is waiting for readyState === 'complete' early enough?

	Axel Hecht [:Pike]
	Comment 9 • 8 years ago

David, can you help us figuring out how to make a11y pick up the changes we make to the DOM at runtime?

The bug took a few turns in its history, but comment 8 reflects the actual question at this point.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 10 • 8 years ago

Eitan can you check this out?

	Eitan Isaacson [:eeejay]
	Comment 11 • 8 years ago

(In reply to Axel Hecht [:Pike] from comment #9)
> David, can you help us figuring out how to make a11y pick up the changes we
> make to the DOM at runtime?
> 

What do you mean by "a11y"? Are you asking if a retranslation changes the value of nsIAccessible.keyboardShortcut, and if not why?

From the comments it looks like you're asking more generally how to get XUL to behave and actually work with a shortcut change.

If its the latter, I think someone more familiar with XUL would have an answer. I have the larch branch built, so I can do some digging if needed.

How do developers typically trigger a retranslation? Are there docs for that?

	Axel Hecht [:Pike]
	Comment 12 • 8 years ago

Retranslation is something we're actually introducing with l20n, so there's no typical way to do that yet. In particular because we don't have corresponding localizations to our code on larch yet.

With our code, the document is actually filled with English (or localized) strings asynchronously, so anything that's assuming that the document doesn't change after the initial parsing can regress. bug 1292527 is a good example on how this impacts platform integration.

We seem to have at least impact on command keys, but there's way more accessibility tech which integrates into our various platforms, so we're hoping for some help from you as you'd know the expected behaviour.

At least command keys, accesskeys and screenreaders come to my mind, not sure if there's more?

	Eitan Isaacson [:eeejay]
	Comment 13 • 8 years ago

Sorry, I don't have the XUL knowledge to help here. Unflagging the ni so I don't block here.