Bug 99578 - [EFL] Share WebKit-Gtk's Accessibility implementation with others WebKit ports.
Summary: [EFL] Share WebKit-Gtk's Accessibility implementation with others WebKit ports.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 101727
Blocks: 98895 99156 100848 101748 102502 105504
  Show dependency treegraph
 
Reported: 2012-10-17 03:04 PDT by Krzysztof Czech
Modified: 2012-12-20 00:49 PST (History)
12 users (show)

See Also:


Attachments
1. Isolate the GTK/Gail/Pango specific code in accessibility/gtk (9.11 KB, patch)
2012-10-26 03:39 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff
Renamed accessibility/gtk to accessibility/atk (33.27 KB, patch)
2012-10-26 03:41 PDT, Mario Sanchez Prada
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Krzysztof Czech 2012-10-17 03:04:22 PDT
WebKit-GTK ensures Accessibility support with ATK library.
WebKit-EFL's implementation will be based on ATK as well. I think it does not make sense to
duplicate the implementation. Core parts of the code could be shared among the ports and differences put inside
specific platform macros. I propose to move the implementation from ./WebCore/accessibility/gtk to ./WebCore/accessibility/atk.
Comment 1 Mario Sanchez Prada 2012-10-17 03:25:41 PDT
Adding Piñeiro and Joanmarie to this. I think they might be interested too.
Comment 2 Alejandro Piñeiro 2012-10-22 04:11:32 PDT
(In reply to comment #0)
> WebKit-GTK ensures Accessibility support with ATK library.
> WebKit-EFL's implementation will be based on ATK as well. I think it does not make sense to
> duplicate the implementation. Core parts of the code could be shared among the ports and differences put inside
> specific platform macros. I propose to move the implementation from ./WebCore/accessibility/gtk to ./WebCore/accessibility/atk.

I agree that sharing the ATK support as much as possible makes total sense. That directory name makes sense to me.

Anyway, I think that the first step could check if there are some kind of any "pure gtk" code on current accessibility/gtk directory. If so, next step would be split that code, and check if we need to maintain a accessibility/gtk directory with that gtk-specific code, or move it to the general gtk code (as I'm thinking on gtk-widget initialization related code)
Comment 3 Mario Sanchez Prada 2012-10-23 01:08:14 PDT
(In reply to comment #2)
> [...]
> Anyway, I think that the first step could check if there are some kind of any
> "pure gtk" code on current accessibility/gtk directory. If so, next step would
> be split that code, and check if we need to maintain a accessibility/gtk
> directory with that gtk-specific code, or move it to the general gtk code

As far as I know, there are a couple of references to GtkWidget in WebKitAccessibleWrapperAtk.cpp and then some usages of GAIL's gail_text_util_get_text() in WebKitAccessibilityInterfaceText.cpp. Other than that, the remaining stuff should be reusable among ports, I think.

> (as I'm thinking on gtk-widget initialization related code)

Not sure what you mean. If you're talking about the initialization of the WebView (which is a GtkWidget) and its implementation of gtk_widget_get_accessible(), such code is not here in WebCore/accessibility/gtk, but in WebKit/gtk/webkit (for WK1) and WebKit2/UIProcess/API/gtk (for WK2).

Btw, forgot to mention it before, but I also agree with sharing this a11y code between the GTK and the EFL ports looks like a good idea
Comment 4 Alejandro Piñeiro 2012-10-23 03:43:30 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > [...]
> > Anyway, I think that the first step could check if there are some kind of any
> > "pure gtk" code on current accessibility/gtk directory. If so, next step would
> > be split that code, and check if we need to maintain a accessibility/gtk
> > directory with that gtk-specific code, or move it to the general gtk code
> 
> As far as I know, there are a couple of references to GtkWidget in WebKitAccessibleWrapperAtk.cpp and then some usages of GAIL's gail_text_util_get_text() in WebKitAccessibilityInterfaceText.cpp. Other than that, the remaining stuff should be reusable among ports, I think.

Hmm, good point. The problematic point would be the usage of gail_text_util methods on the AtkText implementation. FWIW, GTK and clutter are not using them anymore, but using a private fully pango-based library. So another task would be check what EFL is using for the text rendering.

Probably this bug will became a metabug of all the tasks needed to make the move. Your paragraph detects two.

> > (as I'm thinking on gtk-widget initialization related code)
> 
> Not sure what you mean. If you're talking about the initialization of the WebView (which is a GtkWidget) and its implementation of gtk_widget_get_accessible(), such code is not here in WebCore/accessibility/gtk, but in WebKit/gtk/webkit (for WK1) and WebKit2/UIProcess/API/gtk (for WK2).

Yes I was talking about that. So good to know that we can discard my concern as a new task.


> Btw, forgot to mention it before, but I also agree with sharing this a11y code between the GTK and the EFL ports looks like a good idea
Comment 5 Mario Sanchez Prada 2012-10-23 03:59:52 PDT
(In reply to comment #4)
> [...]
> Hmm, good point. The problematic point would be the usage of gail_text_util 
> methods on the AtkText implementation. FWIW, GTK and clutter are not using 
> them anymore, but using a private fully pango-based library.

Yes, which reminds me of another issue with the a11y code here: pango is still being used in some places there and, as far as I know, its usage inside WebKit is discouraged and should be avoided.

I think the right replacement is TextBreakIterator, not 100% sure, though.

> So another task would be check what EFL is using for the text rendering.

Text rendering code is not an issue in WebCore/accessibility/gtk. Pango is being used only to get a PangoLayout to pass to gail_text_util_get_text() so I think getting rid of gail would be enough (as easy as it sounds, as hard as it might be :-))

And AFAIK, the EFL port is already avoiding pango and relying in TextBreakIterator instead, but Krysztof will know better, I suppose.

> Probably this bug will became a metabug of all the tasks needed to make the 
> move. Your paragraph detects two.

Yes, could be.

> > > (as I'm thinking on gtk-widget initialization related code)
> > 
> > Not sure what you mean. If you're talking about the initialization of the 
> > WebView (which is a GtkWidget) and its implementation of 
> > gtk_widget_get_accessible(), such code is not here in 
> > WebCore/accessibility/gtk, but in WebKit/gtk/webkit (for WK1) and 
> > WebKit2/UIProcess/API/gtk (for WK2).
> 
> Yes I was talking about that. So good to know that we can discard my concern
> as a new task.

Ok, great.
Comment 6 Grzegorz Czajkowski 2012-10-24 01:26:36 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > [...]
> > Hmm, good point. The problematic point would be the usage of gail_text_util 
> > methods on the AtkText implementation. FWIW, GTK and clutter are not using 
> > them anymore, but using a private fully pango-based library.
> 
> Yes, which reminds me of another issue with the a11y code here: pango is still being used in some places there and, as far as I know, its usage inside WebKit is discouraged and should be avoided.
> 
> I think the right replacement is TextBreakIterator, not 100% sure, though.

Pango has been finally removed from WebKit-EFL dependencies. We found out all needed replacements for example, the spell checker feature uses TextBreakIterator to determine the beginning/end of the word (bug 94320).

> 
> > So another task would be check what EFL is using for the text rendering.
> 
> Text rendering code is not an issue in WebCore/accessibility/gtk. Pango is being used only to get a PangoLayout to pass to gail_text_util_get_text() so I think getting rid of gail would be enough (as easy as it sounds, as hard as it might be :-))

I completely agree with you. IMHO it looks like  WebCore methods could be used (like TextIterator) instead of gail_text_util_get_text(). The current implementation already uses it and based on Range (WebCore/dom/Range.h) we could manipulate TextIterator object in the same way as gail is doing.
Comment 7 Mario Sanchez Prada 2012-10-26 03:39:08 PDT
Created attachment 170876 [details]
1. Isolate the GTK/Gail/Pango specific code in accessibility/gtk

(In reply to comment #6)
> [...]
> I completely agree with you. IMHO it looks like  WebCore methods could be used (like TextIterator) instead of gail_text_util_get_text(). 
> The current implementation already uses it and based on Range (WebCore/dom/Range.h) we could manipulate TextIterator object in the same
> way as gail is doing.

I'm attaching a quick patch I did by adding some #if PLATFORM(GTK) (and removing 'GTK' from some comments too) as a starting point, in order to show which parts inside accessibility/gtk are really depending on GTK+/Gail/Pango and which ones can be already shared with other ports, as they are just depending on GLib and ATK.

As you can see the big deal is the implementation of atk_text_get_text*() functions, in WebKitAccessibleInterfaceText.cpp, and finding the parent accessible object for the root AtkObject, in WebKitAccessibleWrapperAtk.cpp. For the later, I'm afraid there're no big chances we can reuse things so we probably won't be able to get rid of that "#if PLATFORM" there, but for the atk_text_get_text*() functions I think that the implementation needed for EFL could be reused for GTK too, helping WebKitGtk+ get rid of Pango/Gail dependencies there, which would be a big win.

What do you think? If you think this makes sense, the remaining step for making the patch for this bug complete would be just to rename the directory to atk and update the makefiles.
Comment 8 Mario Sanchez Prada 2012-10-26 03:41:28 PDT
Created attachment 170877 [details]
Renamed accessibility/gtk to accessibility/atk

This would be the other one
Comment 9 Alejandro Piñeiro 2012-10-26 04:33:18 PDT
(In reply to comment #7)
> Created an attachment (id=170876) [details]
> 1. Isolate the GTK/Gail/Pango specific code in accessibility/gtk
> 
> (In reply to comment #6)
> > [...]
> > I completely agree with you. IMHO it looks like  WebCore methods could be used (like TextIterator) instead of gail_text_util_get_text(). 
> > The current implementation already uses it and based on Range (WebCore/dom/Range.h) we could manipulate TextIterator object in the same
> > way as gail is doing.
> 
> I'm attaching a quick patch I did by adding some #if PLATFORM(GTK) (and removing 'GTK' from some comments too) as a starting point, in order to show which parts inside accessibility/gtk are really depending on GTK+/Gail/Pango and which ones can be already shared with other ports, as they are just depending on GLib and ATK.

Seems a good starting point, thanks for it.

> As you can see the big deal is the implementation of atk_text_get_text*() functions, in WebKitAccessibleInterfaceText.cpp, and finding the parent accessible object for the root AtkObject, in WebKitAccessibleWrapperAtk.cpp. For the later, I'm afraid there're no big chances we can reuse things so we probably won't be able to get rid of that "#if PLATFORM" there, but for the atk_text_get_text*() functions I think that the implementation needed for EFL could be reused for GTK too, helping WebKitGtk+ get rid of Pango/Gail dependencies there, which would be a big win.

Yeah, probably it would be hard to remove the GTK specific code in order to get the parent. Anyway, this is just one method, and most of the gtk specific code are the text related methods.

> What do you think? If you think this makes sense, the remaining step for making the patch for this bug complete would be just to rename the directory to atk and update the makefiles.

That is your next patch ;)
Comment 10 Gyuyoung Kim 2012-10-31 18:21:42 PDT
I think you have to change glib data type with common things in order to move from gtk to atk.

For example, 

g_value_init(gValue, G_TYPE_FLOAT);
GValue* gValue
gboolean
g_value_set_float
gchar
g_return_val_if_fail

and so on.

Those files you are moving are using above data types or glib macros. In my humble opinion, we can't move from gtk to atk without this glib dependency.
Comment 11 Gyuyoung Kim 2012-10-31 18:22:22 PDT
(In reply to comment #10)

> Those files you are moving are using above data types or glib macros. In my humble opinion, we can't move from gtk to atk without this glib dependency.

Typo : without -> with.
Comment 12 Martin Robinson 2012-10-31 18:38:33 PDT
(In reply to comment #10)
 
> Those files you are moving are using above data types or glib macros. In my humble opinion, we can't move from gtk to atk without this glib dependency.

ATK itself depends on GLib, so what is the purpose of trying to remove calls to GLib in the ATK accessibility code? Perhaps I'm misunderstanding what you are saying though.
Comment 13 Mario Sanchez Prada 2012-11-02 09:58:24 PDT
(In reply to comment #12)
> (In reply to comment #10)
> 
> > Those files you are moving are using above data types or glib macros. In my humble opinion, we can't move from gtk to atk without this glib dependency.
> 
> ATK itself depends on GLib, so what is the purpose of trying to remove calls to 
> GLib in the ATK accessibility code? Perhaps I'm misunderstanding what you are saying though.

Exactly. I think the confusion might come from mixing GTK with GLib? GTK depends on Glib, exactly in the same way ATK does, and we are talking about going away  from GTK-specific code only, I think, so we should keep those data types around.

Remember that ATK is not just a toolkit-independent definition of interfaces for accessibility related stuff, that is AT-SPI, which is what assistive technologies like screen readers understand in Linux based accessible environments.

However, and according to [1], ATK is "a development toolkit from GNOME which allows programmers to use common GNOME accessibility features", full of GObject interfaces which is what applications/toolkits willing to expose accessibility related information must implement.

Communication between ATK-based accessible applications and AT-SPI based assistive technologies happens thanks to the AT-SPI<->ATK bridge, which makes all the mapping through IPC (D-Bus) between those two worlds.

So, in a nutshell, if the EFL port is willing to reuse the ATK implementation of WebKitGTK+ it needs to keep Glib. Other option is to go ahead and implement their own "ATK-like thing", if it's not there yet, or to go and speak directly with the AT-SPI world from EFL's widget library. That's what they do in Qt with their atspi-qt bridge I think.

[1] http://www.linuxfoundation.org/collaborate/workgroups/accessibility/atk/at-spi
Comment 14 Martin Robinson 2012-11-02 10:40:24 PDT
(In reply to comment #13)

> So, in a nutshell, if the EFL port is willing to reuse the ATK implementation of WebKitGTK+ it needs to keep Glib. Other option is to go ahead and implement their own "ATK-like thing", if it's not there yet, or to go and speak directly with the AT-SPI world from EFL's widget library. That's what they do in Qt with their atspi-qt bridge I think.

It's also important to keep in mind that EFL has an implicit dependency on GLib anyway via the libsoup backend.
Comment 15 Gyuyoung Kim 2012-11-03 23:34:30 PDT
(In reply to comment #14)
> (In reply to comment #13)
> 
> > So, in a nutshell, if the EFL port is willing to reuse the ATK implementation of WebKitGTK+ it needs to keep Glib. Other option is to go ahead and implement their own "ATK-like thing", if it's not there yet, or to go and speak directly with the AT-SPI world from EFL's widget library. That's what they do in Qt with their atspi-qt bridge I think.
> 
> It's also important to keep in mind that EFL has an implicit dependency on GLib anyway via the libsoup backend.

If ATK depends on glib, EFL port also should use glib as libsoup case.  I just thought common module should not have specific port dependency. If this is able to be allowed, I don't mind.


Krzysztof, 

As we know, EFL itself is going to support a library for accessibility. Will you use ATK for EFL port though ?
Comment 16 Krzysztof Czech 2012-11-05 02:44:30 PST
This common module "atk" will not have any specific port dependency, the only dependency is glib, but we are OK with it, regarding what has been said about it.

Well, yes EFL as a framework has some support for Accessibility, but not in terms of ATK and WebKit.
WebKit-GTK had already done that. Mappings to atk are already implemented and as Alejendro said they're are going to be improved. The only thing we (EFL developers) should take care of, are toolkit differences in terms of proper communication with EFL.
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > 
> > > So, in a nutshell, if the EFL port is willing to reuse the ATK implementation of WebKitGTK+ it needs to keep Glib. Other option is to go ahead and implement their own "ATK-like thing", if it's not there yet, or to go and speak directly with the AT-SPI world from EFL's widget library. That's what they do in Qt with their atspi-qt bridge I think.
> > 
> > It's also important to keep in mind that EFL has an implicit dependency on GLib anyway via the libsoup backend.
> 
> If ATK depends on glib, EFL port also should use glib as libsoup case.  I just thought common module should not have specific port dependency. If this is able to be allowed, I don't mind.
> 
> 
> Krzysztof, 
> 
> As we know, EFL itself is going to support a library for accessibility. Will you use ATK for EFL port though ?
Comment 17 Gyuyoung Kim 2012-11-05 02:55:25 PST
(In reply to comment #16)
> This common module "atk" will not have any specific port dependency, the only dependency is glib, but we are OK with it, regarding what has been said about it.

IMO, EFL should not use glib, but we have been used it because there are no alternative solution yet. If EFL supports network library, I think we didn't use libsoup.

> Well, yes EFL as a framework has some support for Accessibility, but not in terms of ATK and WebKit.
> WebKit-GTK had already done that. Mappings to atk are already implemented and as Alejendro said they're are going to be improved. The only thing we (EFL developers) should take care of, are toolkit differences in terms of proper communication with EFL.

Thank you for your explanation. If there is no alternative solution in EFL, I'm ok for now though I don't like to have new glib dependency,
Comment 18 Krzysztof Czech 2012-11-09 01:11:24 PST
> Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:582
> +    notImplemented()

Missing semicolon.

> Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:592
> +    notImplemented()

Missing semicolon.

> Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:246
> +#endif // PLATFORM(GTK)

Compilation error on other platforms.
Should be moved two lines forward.

> Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:248
>          }

#endif // PLATFORM(GTK)
Should be placed here.


Would like me to continue this bug ?. I think it can be already shipped to mainstream. I found only some minor mistakes

Patches do not make any problems on GTK regarding a11y layout tests.
Results are the same before and after the change.

Found 157 tests; running 138, skipping 19.
All 138 tests run as expected.
Comment 19 Mario Sanchez Prada 2012-11-09 02:07:59 PST
(In reply to comment #18)
> > Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:582
> > +    notImplemented()
> 
> Missing semicolon.
> 
> > Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:592
> > +    notImplemented()
> 
> Missing semicolon.
>
> > Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:246
> > +#endif // PLATFORM(GTK)
> 
> Compilation error on other platforms.
> Should be moved two lines forward.

Oops! Sometimes I'm so absentminded... thanks for catching these.

> > Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:248
> >          }
> 
> #endif // PLATFORM(GTK)
> Should be placed here.
> 
> Would like me to continue this bug ?. I think it can be already shipped to 
> mainstream. I found only some minor mistakes

No problem. If the patches are good for you already as they are (as it seems to be), I think I can do it myself. Actually I have filed a new bug for the first part (see bug 101727) and plan to provide an updated patch for the second one (moving from gtk/ to atk/) now.

> Patches do not make any problems on GTK regarding a11y layout tests.
> Results are the same before and after the change.
> 
> Found 157 tests; running 138, skipping 19.
> All 138 tests run as expected.

Great! Happy to help
Comment 20 Mario Sanchez Prada 2012-11-09 02:16:35 PST
Comment on attachment 170877 [details]
Renamed accessibility/gtk to accessibility/atk

(In reply to comment #19)
> [...]
> No problem. If the patches are good for you already as they are (as it 
> seems to be), I think I can do it myself. Actually I have filed a new bug 
> for the first part (see bug 101727) and plan to provide an updated patch 
> for the second one (moving from gtk/ to atk/) now.

Well, actually there's no new patch to attach, since I'm afraid the second one already attached in this bug is the one we would need to get in.

Asking for review then
Comment 21 WebKit Review Bot 2012-11-09 02:22:39 PST
Attachment 170877 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/accessibility/atk/WebKitAccessibleUtil.cpp:167:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleHyperlink.cpp:342:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleHyperlink.cpp:343:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:206:  webkit_accessible_parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:747:  webkit_accessible_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:771:  type_volatile is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:788:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:967:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:968:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:972:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:210:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:217:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.h:49:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.h:52:  webkit_accessible_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 17 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Krzysztof Czech 2012-11-09 02:45:15 PST

(In reply to comment #19)
> (In reply to comment #18)
> > > Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:582
> > > +    notImplemented()
> > 
> > Missing semicolon.
> > 
> > > Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:592
> > > +    notImplemented()
> > 
> > Missing semicolon.
> >
> > > Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:246
> > > +#endif // PLATFORM(GTK)
> > 
> > Compilation error on other platforms.
> > Should be moved two lines forward.
> 
> Oops! Sometimes I'm so absentminded... thanks for catching these.
> 
> > > Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:248
> > >          }
> > 
> > #endif // PLATFORM(GTK)
> > Should be placed here.
> > 
> > Would like me to continue this bug ?. I think it can be already shipped to 
> > mainstream. I found only some minor mistakes
> 
> No problem. If the patches are good for you already as they are (as it seems to be), I think I can do it myself. Actually I have filed a new bug for the first part (see bug 101727) and plan to provide an updated patch for the second one (moving from gtk/ to atk/) now.
Yes, patches are good for me. I've started EFL part (100848). I think WebProcess part can be also shared. I'm on the way to propose a patch.
> 
> > Patches do not make any problems on GTK regarding a11y layout tests.
> > Results are the same before and after the change.
> > 
> > Found 157 tests; running 138, skipping 19.
> > All 138 tests run as expected.
> 
> Great! Happy to help
Comment 23 Mario Sanchez Prada 2012-11-15 04:00:48 PST
(In reply to comment #21)
> Attachment 170877 [details] did not pass style-queue:
> [...] 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

This patch is just about renaming a directory, so I don't think it makes much sense to pay attention to this coding style issues in this case.

Ping reviewers?
Comment 24 Martin Robinson 2012-11-15 08:48:34 PST
Comment on attachment 170877 [details]
Renamed accessibility/gtk to accessibility/atk

Might as well fix the style issues when landing the patch.
Comment 25 Krzysztof Czech 2012-11-15 09:11:50 PST
Mario, could take a look at https://bugs.webkit.org/show_bug.cgi?id=101748
Comment 26 Mario Sanchez Prada 2012-11-16 03:21:34 PST
(In reply to comment #24)
> (From update of attachment 170877 [details])
> Might as well fix the style issues when landing the patch.

:-) Ok, I'll do. They are not that many issues anyway.

Thanks for the review
Comment 27 Mario Sanchez Prada 2012-11-16 03:23:36 PST
(In reply to comment #25)
> Mario, could take a look at https://bugs.webkit.org/show_bug.cgi?id=101748

I see Martin has already r+'ed it, but sure I'll take a look anyway.
Comment 28 Mario Sanchez Prada 2012-11-16 05:47:47 PST
Committed r134939: <http://trac.webkit.org/changeset/134939>