Bug 114354 – [ATK] Adds an accessibility support to access a value of the color control element

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Krzysztof Czech]

Making color control element to be accessible by adding AtkText interface.
Supporting new role "AXColorWell" in WTR and DRT.
Providing new expected results of color-well.html into EFL port.

[Krzysztof Czech]

Created attachment 197732 [details]
Patch

[Mario Sanchez Prada]

I think it would be more accurate to implement AtkValue interface [1] for this kind of input elements, instead of partly implementing AtkText, which seems a bit "forced" to me in this case.

API, joanie... what do you think?

[1] https://developer.gnome.org/atk/unstable/AtkValue.html

[Joanmarie Diggs]

(In reply to comment #2)
> I think it would be more accurate to implement AtkValue interface [1] for this kind of input elements, instead of partly implementing AtkText, which seems a bit "forced" to me in this case.
> 
> API, joanie... what do you think?
> 
> [1] https://developer.gnome.org/atk/unstable/AtkValue.html

While AtkValue's docs suggest the Value can be whatever, AT-SPI2 seems to expect [2] -- and provide [3] -- a double. So I would either go with AtkText or I would change the bridge and the AT-SPI2 API to accommodate string values first.

As for whether or not it is "forced" to go with AtkText, with AtkValue [1] you also need to provide a minimum, a maximum, and increments. What will those be here? (For me, AtkValue is the more "forced" of the two.)

[2] https://git.gnome.org/browse/at-spi2-atk/tree/atk-adaptor/adaptors/value-adaptor.c
[3] https://git.gnome.org/browse/at-spi2-core/tree/atspi/atspi-value.c

[Mario Sanchez Prada]

(In reply to comment #3)
> (In reply to comment #2)
> > I think it would be more accurate to implement AtkValue interface [1] for this kind of input elements, instead of partly implementing AtkText, which seems a bit "forced" to me in this case.
> > 
> > API, joanie... what do you think?
> > 
> > [1] https://developer.gnome.org/atk/unstable/AtkValue.html
> 
> While AtkValue's docs suggest the Value can be whatever, AT-SPI2 seems to 
> expect [2] -- and provide [3] -- a double. So I would either go with AtkText
> or I would change the bridge and the AT-SPI2 API to accommodate string values
> first.

Ah, I see the problem now. However I'm still not convinced whether to expose it through AtkText will be the right thing to do because it will create more questions in my mind related to the implementation of other functions in AtkText (e.g. those related to the position of the caret).

I'm not saying I'm against this change, after all I will basically trust you Joanie way more than myself when it comes to define requirements for ATs, just trying to see the pros and the cons of each solution.
 
> As for whether or not it is "forced" to go with AtkText, with AtkValue [1]
> you also need to provide a minimum, a maximum, and increments. What will
> those be here? (For me, AtkValue is the more "forced" of the two.)

Yes, I already thought about that and there was no easy answer for those cases either, other than the obvious "let's use 0 as minimum, 2^64 as maximum and 1.0 as increment" :-)

[Krzysztof Czech]

Created attachment 210443 [details]
Patch

[Krzysztof Czech]

(In reply to comment #2)
> I think it would be more accurate to implement AtkValue interface [1] for this kind of input elements, instead of partly implementing AtkText, which seems a bit "forced" to me in this case.
> 
> API, joanie... what do you think?
> 

I do not have any other arguments than those what joanie said. I agree with her.

[Krzysztof Czech]

Created attachment 210447 [details]
Patch

[Mario Sanchez Prada]

Comment on attachment 210447 [details]
Patch

(In reply to comment #6)
> (In reply to comment #2)
> > I think it would be more accurate to implement AtkValue interface [1] for this kind of input elements, instead of partly implementing AtkText, which seems a bit "forced" to me in this case.
> > 
> > API, joanie... what do you think?
> > 
> 
> I do not have any other arguments than those what joanie said. I agree with her.

It seems you both agree on that and, on top of that, I also can't see a better solution so I'm fine with that.

We'll probably need to rebaseline the results for GTK too, but that's something we can do once it's landed.

[WebKit Commit Bot]

Comment on attachment 210447 [details]
Patch

Rejecting attachment 210447 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 210447, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/1693587

[Mario Sanchez Prada]

Comment on attachment 210447 [details]
Patch

(In reply to comment #9)
> (From update of attachment 210447 [details])
> Rejecting attachment 210447 [details] from commit-queue.
> 
> Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 210447, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
> 
> ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.
> 
> Full output: http://webkit-queues.appspot.com/results/1693587

Oops! It seems the ChangeLog was kind of duplicated (and I missed that one for some reason, too).

Please fix it and provide a new patch

[Krzysztof Czech]

Created attachment 210450 [details]
Patch

[Krzysztof Czech]

> Oops! It seems the ChangeLog was kind of duplicated (and I missed that one for some reason, too).
> 
> Please fix it and provide a new patch
Done.

[Krzysztof Czech]

Created attachment 210453 [details]
Patch

[Mario Sanchez Prada]

Comment on attachment 210453 [details]
Patch

Thanks. Let's give it another shot then...

[WebKit Commit Bot]

Comment on attachment 210453 [details]
Patch

Clearing flags on attachment: 210453

Committed r155041: <http://trac.webkit.org/changeset/155041>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.