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.
Created attachment 197732 [details] Patch
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
(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
(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" :-)
Created attachment 210443 [details] Patch
(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.
Created attachment 210447 [details] Patch
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.
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
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
Created attachment 210450 [details] Patch
> 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.
Created attachment 210453 [details] Patch
Comment on attachment 210453 [details] Patch Thanks. Let's give it another shot then...
Comment on attachment 210453 [details] Patch Clearing flags on attachment: 210453 Committed r155041: <http://trac.webkit.org/changeset/155041>
All reviewed patches have been landed. Closing bug.