Closed Bug 938164 Opened 6 years ago Closed 6 years ago

Need to implement atk_object_get_object_locale, as atk_document_get_locale is deprecated

Categories

(Core :: Disability Access APIs, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: apinheiro, Assigned: tbsaunde)

Details

(Whiteboard: [qa-])

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130917102914

Steps to reproduce:

Since 2.8 AtkObject includes a method called atk_object_get_object_locale [1], method that replaces atk_document_get_locale. In short, it was needed because locale in some cases is not something constant for all the document, and different objects on the same document can have a different role. More info on the atk bug that handle this issue [2].

FWIW, WebkitGTK already did this. It didn't remove the implementation of atk_document_get_locale in order to support older versions of ATs, so probably that could be done here too.

[1] https://developer.gnome.org/atk/stable/AtkObject.html#atk-object-get-object-locale
[2] https://bugzilla.gnome.org/show_bug.cgi?id=694117
Component: General → Disability Access APIs
Since I want to kill other-licenses/atk-1.0/ and just use the atk headers on the build machine I went for doing a minimal update of the headers, doing a diff of the in tree header and the one in atk git didn't show any obvious binary incompatibility.
Attachment #832089 - Flags: review?(surkov.alexander)
Comment on attachment 832089 [details] [diff] [review]
bug 938164 - implement AtkObject::get_object_locale

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

::: accessible/src/atk/AccessibleWrap.cpp
@@ +156,5 @@
>  /* getDescriptionCB is also used by image interface */
>         const gchar*        getDescriptionCB (AtkObject *aAtkObj);
>  static AtkRole             getRoleCB(AtkObject *aAtkObj);
>  static AtkAttributeSet*    getAttributesCB(AtkObject *aAtkObj);
> +static const gchar* GetLocaleCB(AtkObject*);

nit: aAtkObject (missed 'a')

you use uppercase 'G', all others use lowercase (changing file style?)

@@ +756,5 @@
>    return accWrap ? GetAttributeSet(accWrap) : nullptr;
>  }
>  
> +const gchar*
> +GetLocaleCB(AtkObject* aATKObj)

nit: this file uses aAtkObj (not aATKObj)
Attachment #832089 - Flags: review?(surkov.alexander) → review+
Assignee: nobody → trev.saunders
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(In reply to alexander :surkov from comment #2)
> Comment on attachment 832089 [details] [diff] [review]
> bug 938164 - implement AtkObject::get_object_locale
> 
> Review of attachment 832089 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/atk/AccessibleWrap.cpp
> @@ +156,5 @@
> >  /* getDescriptionCB is also used by image interface */
> >         const gchar*        getDescriptionCB (AtkObject *aAtkObj);
> >  static AtkRole             getRoleCB(AtkObject *aAtkObj);
> >  static AtkAttributeSet*    getAttributesCB(AtkObject *aAtkObj);
> > +static const gchar* GetLocaleCB(AtkObject*);
> 
> nit: aAtkObject (missed 'a')

huh? I just didn't name it since that's not really useful in prototypes.

> you use uppercase 'G', all others use lowercase (changing file style?)

yeah, functions tarting with lower case leter always confuses me

> @@ +756,5 @@
> >    return accWrap ? GetAttributeSet(accWrap) : nullptr;
> >  }
> >  
> > +const gchar*
> > +GetLocaleCB(AtkObject* aATKObj)
> 
> nit: this file uses aAtkObj (not aATKObj)

ok
(In reply to Trevor Saunders (:tbsaunde) from comment #3)

> > > +static const gchar* GetLocaleCB(AtkObject*);
> > 
> > nit: aAtkObject (missed 'a')
> 
> huh? I just didn't name it since that's not really useful in prototypes.

agree but it's about style

> > you use uppercase 'G', all others use lowercase (changing file style?)
> 
> yeah, functions tarting with lower case leter always confuses me

ok, if you really want it, uppercase seems to be preferred over lowercase but I bet Gecko has plenty amount of examples of lowercase usage, btw ATK files is one of them
https://hg.mozilla.org/mozilla-central/rev/fa48287a7eca
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.