| Summary: | [Gtk] Find a way for WebKit to "announce" itself so that ATs can readily distinguish it from true Gtk+/Gail | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joanmarie Diggs <jdiggs> | ||||||
| Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, apinheiro, commit-queue, eric, mario, walker.willie, webkit.review.bot, xan.lopez | ||||||
| Priority: | P2 | Keywords: | Gtk | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 25531 | ||||||||
| Attachments: |
|
||||||||
|
Description
Joanmarie Diggs
2009-10-18 20:55:08 PDT
I haven't tested it, but I suppose that right now, it is returning as toolkit name, GAIL. Right?
As far as I know, the gtk accessibility support in WebKit is not defining a new AtkUtil at all. It is just a new AtkObject that works as a wrapper of the internal a11y support in Webkit.
It is just the a11y support for a specific Gtk Widget, so I don't understand why this require to explicitly say that the toolkit is Webkit (as I really think that we are not in a different toolkit).
I think that you are asking it because in the Gecko implementation, it returns as toolkit "Gecko". But this is different:
* Gecko defines a new AtkUtil implementation (see nsAppRootAccessible)
* It redefines get_root, returning a different root (gail_root if not possible)
* AFAIK, this doesn't happens with Webkit, Webkit always return the gail root.
* It wraps the event management, as the "window:" signal can also be emitted by MaiAtkObject.
* Other issues
So, in this case Gecko is defining a new toolkit (the root is different) that wraps part of their functionality on Gail.
But this is not the case with Webkit. The Webkit a11y support is just implement the Atk interfaces in a new Gtk widget. Technically (IMHO) this doesn't mean a new toolkit, it is just a "library" that defines a new gtk widget that requires a11y support. The main point of that is that Webkit is not defining a new get_root method. So I think that the correct toolkit is GAIL.
I'm missing any reason to consider that we are in a different toolkit?
(In reply to comment #1) > I'm missing any reason to consider that we are in a different toolkit? Yes. :-) For any app for which there is not a specific, custom script in Orca, Orca queries the app to determine what toolkit is being used. How WebKit implements things is in some cases quite different from how the same things would be implemented by a Gtk+ app. If WebKit claims to be Gail, Orca's default script will handle things, and it will handle them wrong. Does this make sense?? (In reply to comment #2) > (In reply to comment #1) > > I'm missing any reason to consider that we are in a different toolkit? > > Yes. :-) > > For any app for which there is not a specific, custom script in Orca, Orca > queries the app to determine what toolkit is being used. How WebKit implements > things is in some cases quite different from how the same things would be > implemented by a Gtk+ app. If WebKit claims to be Gail, Orca's default script > will handle things, and it will handle them wrong. > > Does this make sense?? Yes, know I understand the reasons. Thanks. Anyway, and probably just a nitpick, my concern about redefine toolkit_name and toolkit_version is because, as I said in my previous comment, there isn't a new toolkit here, so redefine the toolkit_name as "Webkit" is just a hack hint for the AT applications (like Orca). But I also understand that from the POV of Orca would be more easy to implement a lot of things, as Webkit will not be tied to one specific application (it would be used for epiphany, devhelp, and so on) so it has sense to share it. The other option, quick-thought, would be have a epiphany, devhelp and so on scripts, all of them calling common python scripts, but the problem is that if it appears a new Gtk+ using WebKitWebView, it wouldn't use the specific Orca scripts without updating Orca. Probably the best option to solve that is just redefine toolkit_name as you suggest, perhaps any others (Xan?) have their own opinion. (In reply to comment #3) > The other option, quick-thought, would be have a epiphany, devhelp and so on > scripts, all of them calling common python scripts, Yup, we do that sort of thing in Orca now. > but the problem is that if > it appears a new Gtk+ using WebKitWebView, it wouldn't use the specific Orca > scripts without updating Orca. Exactly. > Probably the best option to solve that is just redefine toolkit_name as you > suggest, perhaps any others (Xan?) have their own opinion. The other option is to make WebKit act more Gailish. The biggest issue is the fact that text is rendered as a child (and in some cases multiple children) of HTMLElement. (In reply to comment #2) > (In reply to comment #1) > > I'm missing any reason to consider that we are in a different toolkit? > > Yes. :-) > > For any app for which there is not a specific, custom script in Orca, Orca > queries the app to determine what toolkit is being used. How WebKit implements > things is in some cases quite different from how the same things would be > implemented by a Gtk+ app. If WebKit claims to be Gail, Orca's default script > will handle things, and it will handle them wrong. > > Does this make sense?? I think the only thing that matters here is what the de-facto expected behavior is, so if Orca and others expect widgets within the WebView hierarchy to report something other than GTK+ as toolkit name, I'm fine with it. I was thinking a little about that, and, although probably I should check this deeply, right now I think that the problem is that this is not a easy change, and this would have several consequences. In order to redefine this two methods you will need to create a new AtkUtil implementation. This is not just implement a new interface, is a subclass of AtkUtil, and it is designed to be a singleton class (so you can't implement it on the current atk wrapper, a new object is required). This means that probably you will need to redefine all the AtkUtil methods in order to wrap properly the gail_util implementation of AtkUtil, and then redefine this two methods. And of course, this class needs to be loaded in order to redefine all this methods, and this is required to be done *after* GAIL to overwrite properly the methods. We all know all the problems we had to load the specific a11y thingies on Firefox, ie: http://mail.gnome.org/archives/gnome-accessibility-list/2009-January/msg00030.html How this class will be loaded? * There is something like webkit_init ()? Could be used? * A specific module for webkit a11y (WAIL)? * On the application side? (Check nsApplicationAccessibleWrap::Init() on nsAppRootAccessible.cpp) * This would be a mess with several applications using Webkit Note to self: Given what a mess this is starting to sound like, we could alternatively cause WebKit to "announce itself" via an object attribute placed on its top-level object. Or something along those lines. I'll give it some more thought. Resummarizing. I was just chatting with Will Walker about this issue. He suggested such an object attribute be added to the Atk API. Enhancement request filed here: https://bugzilla.gnome.org/show_bug.cgi?id=598952. Once we know what the attribute's name is, we can add it to WebKit. :-) Created attachment 50663 [details]
new object attribute - includes layout test
Nothing has happened to the GNOME bug I filed; in the meantime, Orca (and potentially other ATs) still have the need to know if a given object is a WebKitGtk object.
I discussed this further with Will, and he suggested an object attribute of 'toolkit:WebKitGtk'. Other toolkits (e.g. Gecko) can follow our lead. :-)
Note: This has no impact on the reported toolkit for the application; merely gives ATs a hint about where the object is coming from.
Comment on attachment 50663 [details]
new object attribute - includes layout test
Clearing the review? flag. Xan pointed out how my list iteration could be done better. (Thanks!)
Created attachment 50943 [details]
new object attribute - includes layout test
Hopefully this is the right way to do things. :-)
Comment on attachment 50943 [details]
new object attribute - includes layout test
Thanks, this looks great now.
Comment on attachment 50943 [details] new object attribute - includes layout test Rejecting patch 50943 from commit-queue. Unexpected failure when landing patch! Please file a bug against webkit-patch. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 50943, '--parent-command=commit-queue', '--no-update']" exit_code: 1 Last 500 characters of output: 0943&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=30500&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Processing patch 50943 from bug 30500. NOBODY (OOPS!) found in /Users/eseidel/Projects/CommitQueue/LayoutTests/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /Users/eseidel/Projects/CommitQueue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Comment on attachment 50943 [details]
new object attribute - includes layout test
Pretty please?
Comment on attachment 50943 [details] new object attribute - includes layout test Rejecting patch 50943 from commit-queue. Unexpected failure when landing patch! Please file a bug against webkit-patch. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 50943, '--parent-command=commit-queue', '--no-update']" exit_code: 1 Last 500 characters of output: 0943&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=30500&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Processing patch 50943 from bug 30500. NOBODY (OOPS!) found in /Users/eseidel/Projects/CommitQueue/LayoutTests/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /Users/eseidel/Projects/CommitQueue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Comment on attachment 50943 [details]
new object attribute - includes layout test
Try again.
Comment on attachment 50943 [details] new object attribute - includes layout test Clearing flags on attachment: 50943 Committed r59532: <http://trac.webkit.org/changeset/59532> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/59532 might have broken GTK Linux 32-bit Release The following changes are on the blame list: http://trac.webkit.org/changeset/59530 http://trac.webkit.org/changeset/59531 http://trac.webkit.org/changeset/59532 http://trac.webkit.org/changeset/59533 (In reply to comment #19) > http://trac.webkit.org/changeset/59532 might have broken GTK Linux 32-bit Release > The following changes are on the blame list: > http://trac.webkit.org/changeset/59530 > http://trac.webkit.org/changeset/59531 > http://trac.webkit.org/changeset/59532 > http://trac.webkit.org/changeset/59533 Maybe.... But I doubt it. * This change adds an attribute onto the AtkObject * (To my knowledge) none of our existing tests were checking object attributes, so this isn't introducing anything new which might break a test. * This change implements a previously-unimplemented function in our DRT * (To my knowledge) none of our existing tests were calling that function because it wasn't implemented. :-) Having said that, I've been known to be wrong before. If it's my fault, let me know. |