After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 684576 - API Additions: atk_value_get_description() and atk_value_set_description()
API Additions: atk_value_get_description() and atk_value_set_description()
Status: RESOLVED FIXED
Product: atk
Classification: Platform
Component: atk
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: ATK maintainer(s)
ATK maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-09-21 17:47 UTC by Joanmarie Diggs (IRC: joanie)
Modified: 2014-03-05 18:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
AtkValue refactoring (28.73 KB, patch)
2014-02-22 17:47 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
none Details | Review
A new test on atk/tests implementing the new stuff at AtkValue (10.57 KB, patch)
2014-02-22 17:48 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
none Details | Review

Description Joanmarie Diggs (IRC: joanie) 2012-09-21 17:47:49 UTC
Consider, for instance, a slider or spin button or even a custom widget for rating songs in a media player:

1 == sucks
2 == meh
3 == ok
4 == cool
5 == awesome

ATK only allows the exposure of the numerical data.

One could, hypothetically, take the extra steps to make the value description the AtkObject's description. However that removes the possibility of describing the object itself independent of the value. And it seems like a hackaround anyway: Since the description is a textual representation of the current value, one should be able to get it (and I assume set it) via AtkValue.
Comment 1 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-09-22 16:49:01 UTC
Just to understand a little more this proposal:

* For that specific example, what atk_value_get_description would return? The possible values of each value ("sucks meh ok cool awesome") or something descriptive ("The value exposes how awesome this song is")? What atk_object_get_description would return?

* Have you seen any similar API doing that? (in order to get ideas, and use cases)
Comment 2 Joanmarie Diggs (IRC: joanie) 2012-09-22 18:07:06 UTC
(In reply to comment #1)
> Just to understand a little more this proposal:
> 
> * For that specific example, what atk_value_get_description would return?

A string. If the numerical value is 3, atk_value_get_description would return "ok".

> descriptive ("The value exposes how awesome this song is")? What
> atk_object_get_description would return?

A description of the object itself, like "How awesome this song is". Or "Star rating widget". Or perhaps nothing. I see these two things as independent. The point is: "Sucks" ... "Awesome" are *values* which are intended to be exposed to users and AtkValue assumes all values are numeric.

> * Have you seen any similar API doing that? (in order to get ideas, and use
> cases)

I haven't yet dug for an API which does it. What prompted me to file this bug is  WebKit's accessibility layout tests include returning the value description. So I'm guessing at least one API is out there which could be used as a starting point for considering how we wish to implement it in ATK.

Having said that, with respect to use cases, the example in the opening report is something I consider valid. From my old JAWS-teaching days, I remember Windows had a security slider which had values akin to "off", "low", "medium", "high", "defcon 1". Given enough time, I could probably think of other things I have seen in the wild which fit this need.
Comment 3 Joanmarie Diggs (IRC: joanie) 2012-09-22 18:20:55 UTC
See also: https://developer.mozilla.org/en-US/docs/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-valuetext_attribute

Web content creators can do it; we cannot expose it.
Comment 4 Joanmarie Diggs (IRC: joanie) 2013-09-17 10:53:03 UTC
And now, we're going to get it as an AtkObject attribute rather than where it belongs. :(

Let's add this to ATK now.
Comment 5 Joanmarie Diggs (IRC: joanie) 2013-09-17 10:57:56 UTC
(In reply to comment #4)
> And now, we're going to get it as an AtkObject attribute rather than where it
> belongs. :(

https://bugs.webkit.org/show_bug.cgi?id=121477

(Sorry for the spam)
Comment 6 Alejandro Piñeiro Iglesias (IRC: infapi00) 2013-09-17 11:41:02 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > And now, we're going to get it as an AtkObject attribute rather than where it
> > belongs. :(
> 
> https://bugs.webkit.org/show_bug.cgi?id=121477
> 
> (Sorry for the spam)

FWIW, after my work on bug 648623, where I needed to revisit AtkValue, I concluded that AtkValue needs to be rethinked, and I was planning to add this as a 3.12 task. 

In that sense, right now AtkValue uses GValue to return its value, and I think that was a "shy" attempt to support non-numerical values, as GValue allows "any value". But in the end, on at-spi GValue is not supported, so that flexibility is lost. I think that was due problems to represent GValue through an IPC, or just for the fact of needing to guess which type GValue represents. Funnily enough, the problems to represent GValue through gobject-introspection had as a result a over-complicated implementation on bug 648623.

One could suggest that one way to support textual values is just filling the GValue with the text value, and just fix AT-SPI in order to support that. But the fact is that, AFAIK, in some cases you need both, the numerical and the text data.

As I said, I already wrote down this bug (and improve AtkValue in general) on my list for short-term addition on ATK.
Comment 7 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-02-22 17:47:19 UTC
Created attachment 269999 [details] [review]
AtkValue refactoring

In summary this:
   * Stop to use GValue to get/set the value and use doubles instead
   * Include the support for a string description and subranges

There are a lot of changes and deprecations, and is somewhat complex. The big question is how to represent the description of any subrange, if with something symbolic, that ATs can/should interpret, or just as description strings. As right now it is not clear how ATs would interpret any symbolic information that receives, this patch sets a string description for subranges. But that can change in the future*

Advice: this patch includes a lot and detailed documentation for AtkValue. Compile atk with --enable-gtk-doc, and start reading the description of the interface.

* For that same reason, probably the final form of the patch will have AtkRange as a full gobject with set/get methods, instead of a struct where everything is public, but I wanted to share a first working proposal.
Comment 8 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-02-22 17:48:43 UTC
Created attachment 270000 [details] [review]
A new test on atk/tests implementing the new stuff at AtkValue

This patch adds a new test that do a full implementation of AtkValue on a "volume slider, and simulate the user changing the value of this slider.
Comment 9 Mario Sánchez Prada 2014-02-24 14:37:44 UTC
(In reply to comment #7)
> Created an attachment (id=269999) [details] [review]
> AtkValue refactoring
> 
> In summary this:
>    * Stop to use GValue to get/set the value and use doubles instead
>    * Include the support for a string description and subranges
> 
> There are a lot of changes and deprecations, and is somewhat complex. The big
> question is how to represent the description of any subrange, if with something
> symbolic, that ATs can/should interpret, or just as description strings. As
> right now it is not clear how ATs would interpret any symbolic information that
> receives, this patch sets a string description for subranges. But that can
> change in the future*
> 
I haven't compiled this patch but it looks to me like a nice improvement over the current situation with the AtkValue interface. Still, I have some doubts, mainly with the enumeration with default values (e.g. ATK_VALUE_VERY_WEAK):

It seems quite arbitrary to me and, even though I agree all the proposed values in there probably make sense, I wonder wheter it would not be better to provide a reduced set as much generic as possible + a way for the implementor to extend it with custom values/localizedDescriptions if needed.

For instance, provide just the following values:

  ATK_VALUE_SUBSUBOPTIMAL
  ATK_VALUE_SUBOPTIMAL
  ATK_VALUE_OPTIMAL
  ATK_VALUE_VERY_GOOD
  ATK_VALUE_BEST

... with their corresponding localized descriptions, and then a way for implementors to add custom values (out of the range 0-4) with their localized descriptions. Or maybe just to keep those 5 values as "fixed ones" and simple provide implementors with a way to override the desired localized description.

> * For that same reason, probably the final form of the patch will have AtkRange
> as a full gobject with set/get methods, instead of a struct where everything is
> public, but I wanted to share a first working proposal.

I know encapsulation is good, but I personally don't see this particular AtkRange struct as that evil as to require a full-fledged GObject for it. What about just using a boxed type?

Last, I don't think you need/want to export atkprivate.h in Makefile.am, do you? :)

Hope this comments are useful
Comment 10 Mario Sánchez Prada 2014-02-24 14:38:19 UTC
(In reply to comment #8)
> Created an attachment (id=270000) [details] [review]
> A new test on atk/tests implementing the new stuff at AtkValue
> 
> This patch adds a new test that do a full implementation of AtkValue on a
> "volume slider, and simulate the user changing the value of this slider.

Quick comment: I think you want to put 2014 in the copyright header for testvalue.c :)
Comment 11 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-02-25 16:24:19 UTC
(In reply to comment #9)

> I haven't compiled this patch but it looks to me like a nice improvement over
> the current situation with the AtkValue interface.

Thanks.

> Still, I have some doubts,
> mainly with the enumeration with default values (e.g. ATK_VALUE_VERY_WEAK):
> 
> It seems quite arbitrary to me and,

Yes right now it is somewhat arbitrary. I forgot to mention that probably the final list would be smaller. Also SUBSUBOPTIMAL sounds really weird.

> even though I agree all the proposed values
> in there probably make sense, I wonder wheter it would not be better to provide
> a reduced set as much generic as possible + a way for the implementor to extend
> it with custom values/localizedDescriptions if needed.
> 
> For instance, provide just the following values:
> 
>   ATK_VALUE_SUBSUBOPTIMAL
>   ATK_VALUE_SUBOPTIMAL
>   ATK_VALUE_OPTIMAL
>   ATK_VALUE_VERY_GOOD
>   ATK_VALUE_BEST
> 
> ... with their corresponding localized descriptions, and then a way for
> implementors to add custom values (out of the range 0-4) with their localized
> descriptions. Or maybe just to keep those 5 values as "fixed ones" and simple
> provide implementors with a way to override the desired localized description.

It is not needed to add a way to implementors to add custom values. Please, as soon as you compile the patch, read the detailed documentation. Then look at the testvalue example. 

The purpose of those enums are providing default descriptions, and a easy way to get a localized descriptor with atk_value_type_get_name. But the value of the enum itself would not be exposed. Both the new value-changed signal, or the AtkRange fields, are just strings. So if a implementor wants to emit a custom descriptors (ie: the ones at comment 0 ["sucks", "meh", etc]) they just need to fill their AtkRange with that value, and emit signals using "sucks". atk_value_type_get_name is just a utility method, but is not mandatory to be used. That was added because if an implementor decides to do that, the responsibility of emitting localized strings also relies on it. The advantage of GNOME is that it already have a translation team. 

> > * For that same reason, probably the final form of the patch will have AtkRange
> > as a full gobject with set/get methods, instead of a struct where everything is
> > public, but I wanted to share a first working proposal.
> 
> I know encapsulation is good, but I personally don't see this particular
> AtkRange struct as that evil as to require a full-fledged GObject for it. What
> about just using a boxed type?

It is already a boxed type ;)

That doesn't change the fact that right now all this somewhat uncertain. It is not only about encapsulation, but about the possibility to have a private structure and methods to access to the information. Methods that we could deprecate in the future. In general it is easier to deprecate methods that to deprecate struct (data) fields. And we already have that problem right now (bug 653246), I wouldn't like to make that worse.

> 
> Last, I don't think you need/want to export atkprivate.h in Makefile.am, do
> you? :)
> 
> Hope this comments are useful

It is a starting point to the debate.
Comment 12 Joseph Scheuhammer 2014-02-27 15:37:52 UTC
Suggest change to atk_object_set_valuetext(), atk_object_get_valuetext() to align with aria-valuenow vs. aria-valutext distinction.

http://www.w3.org/TR/2014/PR-wai-aria-20140206/states_and_properties#aria-valuetext

Example from the ARIA spec:

"For example, a slider may have rendered values of small, medium, and large. In this case, the values of aria-valuenow could range from 1 through 3, which indicate the position of each value in the value space, but the aria-valuetext would be one of the strings: small, medium, or large."
Comment 13 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-02-27 16:53:26 UTC
(In reply to comment #12)

After re-reading (my own) patch, I would like to mention that I said some wrong stuff at the a11y meeting that raised this questions. On the meeting we were talking about text vs description, assuming that the string equivalent to "value" was called "description". But that is not the case, we are already using "text". See:

On the signal:

       * AtkValue::value-changed:
       * @atkvalue: the object on which the signal was emitted.
       * @value: the new value in a numerical form.
       * @text: human readable text alternative (also called
       * description) of this object. NULL if not available.

Getting the value:

 * atk_value_get_value_and_text:
 * @obj: a GObject instance that implements AtkValueIface
 * @value: (out): address of #gdouble to put the current value of @obj
 * @text: (out) (allow-none): address of #gchar to put the human
 * readable text alternative for @value

So in that aspect, we are already being "aria-alike"

So, where do we have that "description" field? With subranges. So with the example at comment 8, we have an volume slider with the following subranges:
  (0  , 0.2, "low") 
  (0.2, 0.4, "medium")
  (0.4, 0.8, "high"
  (0.8, 1.0, "very high")

And in that case, I see those "low", "medium" etc as description of those ranges. Calling that the "text" of the range sounds odd to me.

FWIW, for the subrange support, I think that aria doesn't have a equivalent yet.

> Suggest change to atk_object_set_valuetext(), atk_object_get_valuetext() to
> align with aria-valuenow vs. aria-valutext distinction.

FWIW, we don't have an atk_value_set_value_text method. We can only get it (via atk_value_get_value_and_text). Sincerely, I don't see the need to this method, as getting "text" should be something that is already part of the information of the object, and not something someone assign previously.

> http://www.w3.org/TR/2014/PR-wai-aria-20140206/states_and_properties#aria-valuetext
> 
> Example from the ARIA spec:
> 
> "For example, a slider may have rendered values of small, medium, and large. In
> this case, the values of aria-valuenow could range from 1 through 3, which
> indicate the position of each value in the value space, but the aria-valuetext
> would be one of the strings: small, medium, or large."

As I said, we are already using "text" instead of "description" to name the string representation of the value. So we are already "aria-alike".

Thanks for the feedback.
Comment 14 Joseph Scheuhammer 2014-02-27 17:06:15 UTC
(In reply to comment #13)

Thanks for the explanation, API, especially as to where the "value text" is (it's in the AtkValue::value-changed signal, and there is a getter for it, "atk_value_get_value_and_text()".

Makes sense to me.
Comment 15 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-03-05 18:08:03 UTC
Just commited two patches, that are basically the previous patches. Differences:

   * The available list of ValueType is smaller. Specifically I removed the subsuboptimal, optimal, etc, as I think that overlaps with very_bad, bad, good, very_good. I didn't remove the others as I think that they are common enough and different enough to be kept.
   * AtkRange is still an boxed type, but the structure is opaque, so right now you need to use accessors in order to get their content (see testvalue to see an example).