405747 - implement IAccessible2::locale

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 1

[Aaron Leventhal]

Comment on attachment 290503 [details] [diff] [review]
patch

Can you add some comments for each block and then request sr=neil?
For example:
// Format: XX-xx
// We just want the XX which represents the language -- we don't need the specific country

Using nsDependantSubstring is probably more efficient than nsAutoString foo(Substring(...))

Comment 2

[alexander :surkov (:asurkov)]

Attachment: patch2

I can't use nsDependentSubstring at least because it hasn't get() method. Probably Neil has an idea.

Comment 3

[[e-mail]]

Use SysAllocStringLen?

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)
> Use SysAllocStringLen?
> 

for the first case only?

Comment 5

[Aaron Leventhal]

Neil isn't CC'd so he probably didn't see your question in comment 4.

Comment 6

[[e-mail]]

(In reply to comment #4)
> (In reply to comment #3)
> > Use SysAllocStringLen?
> for the first case only?
For at least the second and fourth cases, in order to fix comment #2, but you always know the length, so you can use it for the other cases too.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Use SysAllocStringLen?
> > for the first case only?
> For at least the second and fourth cases, in order to fix comment #2, but you
> always know the length, so you can use it for the other cases too.
> 

Do you count cases by SysAllocString entries? Then I do get for the second case, but I didn't catch about fourth and other cases. Or am I able to get pointer on certain char by someway in order to use SysAllocStringLen?

Comment 8

[[e-mail]]

Comment on attachment 290552 [details] [diff] [review]
patch2

>+      nsAutoString country(Substring(lang, 3, 5));
>+      aLocale->country = ::SysAllocString(country.get());
For instance, ::SysAllocStringLen(lang.get() + 3, 2);

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch3

excellent, thanks Neil

Comment 10

[[e-mail]]

Comment on attachment 290674 [details] [diff] [review]
patch3

>+    if (lang.Length() == 2) {
>+      aLocale->language = ::SysAllocString(lang.get());
>+      return S_OK;
>+    }
You could change this to ::SysAllocStringLen(lang.get(), 2);

>+      if (lang.Length() == 5) {
>+        aLocale->country = ::SysAllocStringLen(lang.get() + 3, 2);
>+        return S_OK;
>+      }
Or you could leave this as ::SysAllocString(lang.get() + 3);

Comment 11

[alexander :surkov (:asurkov)]

checked in with Neil's comment (option 2).