[Webkit-unassigned] [Bug 72589] [GTK] Expose accessibility hierarchy in WebKit2 to ATK/AT-SPI based ATs

[bugzilla-daemon at webkit.org]

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


Mario Sanchez Prada <msanchez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #121688|0                           |1
        is obsolete|                            |
 Attachment #121688|review?                     |
               Flag|                            |
 Attachment #121814|                            |review?
               Flag|                            |




--- Comment #30 from Mario Sanchez Prada <msanchez at igalia.com>  2012-01-10 02:16:11 PST ---
Created an attachment (id=121814)
 --> (https://bugs.webkit.org/attachment.cgi?id=121814&action=review)
Patch proposal

(In reply to comment #29)
> (From update of attachment 121688 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121688&action=review
> 
> It looks good to me, great work!. I have just a few more comments, sorry :-P

No worries. Everytime you point something out the patch gets better, so I'm more than happy to address those issues.

Now attaching a new patch taking care of these new ones. See my comments below...

> > Source/WebKit2/UIProcess/API/gtk/tests/AccessibilityTestServer.cpp:33
> > +int
> > +main(int argc, char** argv)
> 
> This should be one line.

Fixed.

> > Tools/Scripts/run-gtk-tests:77
> > +        for path in paths_to_check:
> > +            filepath = os.path.join(exec_prefix, path, filename)
> > +            if os.path.isfile(filepath):
> > +                return filepath
> > +            return None
> 
> You are only checking the first path, the return should be after the loop.

Argh! Dammed python indentation thing... Fixed.

> > Tools/Scripts/run-gtk-tests:97
> > +        Executive().popen(["xprop", "-root", "-remove", "AT_SPI_BUS"],
> > +                          env=test_env, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
> 
> Don't use pipes if you are not going to read/write from/to them

Fixed. I also switched to run_command() here since for 'xprop' I won't need the process later on to kill it (it's not a daemon).

> > Tools/Scripts/run-gtk-tests:105
> > +                a11y_bus_launcher = Executive().popen([a11y_bus_launcher_path],
> > +                                                      env=test_env, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
> 
> Ditto.

Fixed. In this case I kept using popen() because I need to make sure I kill the process later on.

> > Tools/Scripts/run-gtk-tests:116
> > +                a11y_registryd = Executive().popen(["%s/at-spi2-registryd" % atspi2_exec_path],
> > +                                                   env=test_env, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
> 
> Ditto.

Fixed. In this case I kept using popen() because I need to make sure I kill the process later on.

Also, I fixed another bug in there: 

   Bad:
     Executive().popen(["%s/at-spi2-registryd" % atspi2_exec_path] ...

  Good:
     Executive().popen([a11y_registryd_path] ...

Last, I added another modification to this patch, to avoid the following kind of errors when generating the docs:

  Generating WebKit2 documentation...
  ./webkit2gtk-unused.txt:1: warning: 11 unused declarations.They should be added to webkit2gtk-sections.txt in the appropriate place.
  ./webkit2gtk-unused.txt:1: warning: 11 unused declarations.They should be added to webkit2gtk-sections.txt in the appropriate place.

   gtkdoc did not build without warnings

The modification is as follows:

diff --git a/Tools/gtk/generate-gtkdoc b/Tools/gtk/generate-gtkdoc
index db5f83a..0c18a38 100755
--- a/Tools/gtk/generate-gtkdoc
+++ b/Tools/gtk/generate-gtkdoc
@@ -62,6 +62,7 @@ def get_webkit2_options():
                          glob.glob(src_path('PageClientImpl.*')) + \
                          glob.glob(src_path('WebKitUIClient.*')) + \
                          glob.glob(src_path('WebKitWebLoaderClient.*')) + \
+                         glob.glob(src_path('WebKitWebViewBaseAccessible.*')) + \
                          glob.glob(src_path('tests/*.h'))
     })
     return (common.build_path('Source', 'WebKit2', 'webkit2gtk-3.0.pc'), options)


That's all I think. Thanks!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.