751428 - [AccessFu] Remove spaces and catch exceptions when localizing role names

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

Some role names have spaces, we need to remove the spaces to make it a legal token for localization. Also, use an empty string when the role name is not localizable instead of throwing an exception.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Remove spaces and catch exceptions when localizing role names.

Comment 2

[Eitan Isaacson [:eeejay]]

Comment on attachment 620532 [details] [diff] [review]
Remove spaces and catch exceptions when localizing role names.

Oops

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Remove spaces and catch exceptions when localizing role names.

Made a special new method for this, instead of doing the try/catch dance everywhere.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 620536 [details] [diff] [review]
Remove spaces and catch exceptions when localizing role names.

Review of attachment 620536 [details] [diff] [review]:
-----------------------------------------------------------------

r=me. Thanks for the explanation over on bug 751435 and IRC - that you need to trim whitespace so that you can use these as the key for l10n lookups.

::: accessible/src/jsat/UtteranceGenerator.jsm
@@ +220,5 @@
>        return [desc, name];
>      }
> +  },
> +
> +  _getLocalizedRole: function(aRoleStr) {

Are you using this convention for denoting mock encapsulation?

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

(I mean the underscore prefix)

Comment 6

[Eitan Isaacson [:eeejay]]

(In reply to David Bolter [:davidb] from comment #4)
> Comment on attachment 620536 [details] [diff] [review]
> Remove spaces and catch exceptions when localizing role names.
> 
> Review of attachment 620536 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me. Thanks for the explanation over on bug 751435 and IRC - that you need
> to trim whitespace so that you can use these as the key for l10n lookups.

Thanks!

> 
> ::: accessible/src/jsat/UtteranceGenerator.jsm
> @@ +220,5 @@
> >        return [desc, name];
> >      }
> > +  },
> > +
> > +  _getLocalizedRole: function(aRoleStr) {
> 
> Are you using this convention for denoting mock encapsulation?

No, I am using it for private methods on an object that otherwise has public methods. Something from Python, although I have seen it in other mozilla js. Need to be more consistent with it.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to Eitan Isaacson [:eeejay] from comment #6)
> (In reply to David Bolter [:davidb] from comment #4)
> > > +  _getLocalizedRole: function(aRoleStr) {
> > 
> > Are you using this convention for denoting mock encapsulation?
> 
> No, I am using it for private methods on an object that otherwise has public
> methods. Something from Python, although I have seen it in other mozilla js.
> Need to be more consistent with it.

Heh. Encapsulation has two meanings (who knew?) but I meant "a language mechanism for restricting access to some of the object's components." (wikipedia) Anyways, all good. As long as you plan on consistency using _ for mock privacy it is fine with me. It isn't worth doing the Crockford tricks for actual privacy IMO.

Comment 8

[alexander :surkov (:asurkov)]

Comment on attachment 620536 [details] [diff] [review]
Remove spaces and catch exceptions when localizing role names.

Review of attachment 620536 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/jsat/UtteranceGenerator.jsm
@@ +220,5 @@
>        return [desc, name];
>      }
> +  },
> +
> +  _getLocalizedRole: function(aRoleStr) {

pls don't use unnamed functions

@@ +222,5 @@
> +  },
> +
> +  _getLocalizedRole: function(aRoleStr) {
> +    try {
> +      return gStringBundle.GetStringFromName(aRoleStr.replace(' ', ''));

just curious if properties file is updated to this logic already

Comment 9

[Eitan Isaacson [:eeejay]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/c608de1b6a53

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c608de1b6a53