The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

Trunk
mozilla15
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 620532 [details] [diff] [review]
Remove spaces and catch exceptions when localizing role names.
(Assignee)

Comment 2

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

Oops
Attachment #620532 - Attachment is obsolete: true
Attachment #620532 - Flags: review?
(Assignee)

Comment 3

5 years ago
Created attachment 620536 [details] [diff] [review]
Remove spaces and catch exceptions when localizing role names.

Made a special new method for this, instead of doing the try/catch dance everywhere.
Attachment #620536 - Flags: review?(dbolter)

Updated

5 years ago
Attachment #620532 - Flags: review?
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?
Attachment #620536 - Flags: review?(dbolter) → review+
(I mean the underscore prefix)
(Assignee)

Comment 6

5 years ago
(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.
(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

5 years ago
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
(Assignee)

Comment 9

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/c608de1b6a53
Assignee: nobody → eitan
OS: Linux → Android
Hardware: x86_64 → ARM
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/c608de1b6a53
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.