Bug 72708 - [GTK] Accessibility API tests failing because of using non-WebKit GtkWidgets
Summary: [GTK] Accessibility API tests failing because of using non-WebKit GtkWidgets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2011-11-18 02:54 PST by Mario Sanchez Prada
Modified: 2011-11-18 08:47 PST (History)
1 user (show)

See Also:


Attachments
Patch proposal (6.54 KB, patch)
2011-11-18 02:58 PST, Mario Sanchez Prada
xan.lopez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2011-11-18 02:54:21 PST
The ATK unit tests testWebkitAtkParentForRootObject() and testWebkitAtkSetParentForObject() has been failing in the bots lately and it seems it's because they are the only one using GtkWidget's not provided by WebKit (GtkWindow and GtkBox), so when the test asks for the AtkObject's associated to them (through gtk_widget_get_accessible()) the returned object is not the expected one, but a dummy AtkNoOpObject, since the accessibility implementation of Gtk is not being loaded along with the tests.

And this results on errors like this:

  TEST: /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/Programs/unittests/testatk... (pid=25379)
  **
  ERROR:../../Source/WebKit/gtk/tests/testatk.c:1629:testWebkitAtkParentForRootObject: assertion failed: (axBoxChild)

One possible solution is to only use GtkWidget's provided by WebKitGTK+ instead of GtkWindow and GtkBox, to use them as parent containers. And this is a very good option since WebKitWebView widget inherits from GtkContainer, so that makes it suitable for that task.

Will attach a patch soon.
Comment 1 Mario Sanchez Prada 2011-11-18 02:58:04 PST
Created attachment 115774 [details]
Patch proposal
Comment 2 Xan Lopez 2011-11-18 03:38:44 PST
Comment on attachment 115774 [details]
Patch proposal

So, this is a pretty big hack.

I think it's much more interesting to try to figure out why the a11y stuff is not being loaded, since that will likely cause us more problems in the future.

That being said, since Mario says he cannot figure out atm and in the interest of keeping the bots green I'll r+ this patch in case he wants to land it to investigate things properly.
Comment 3 Mario Sanchez Prada 2011-11-18 03:41:38 PST
(In reply to comment #2)
> (From update of attachment 115774 [details])
> So, this is a pretty big hack.

A webView inside another webView? Oh, yeah it is sure a hack... but the best thing I could find at the moment, since my investigations in the bots about why loading the module would not work have no shed any light yet...

> I think it's much more interesting to try to figure out why the a11y stuff is not being loaded, since that will 
> likely cause us more problems in the future.

Agreed.

> That being said, since Mario says he cannot figure out atm and in the interest of keeping the bots green I'll r+ 
> this patch in case he wants to land it to investigate things properly.

Thanks, I will push this now then and will try to figure out what's the true reason and a better solution asap.
Comment 4 Mario Sanchez Prada 2011-11-18 03:46:32 PST
Committed r100747: <http://trac.webkit.org/changeset/100747>
Comment 5 Mario Sanchez Prada 2011-11-18 08:47:32 PST
(In reply to comment #3)
> (In reply to comment #2)
> [...]
> > I think it's much more interesting to try to figure out why the a11y stuff is not being loaded, since that will 
> > likely cause us more problems in the future.
> 
> Agreed.

I found the root issue here. See bug 72732 for the details :-)

> > That being said, since Mario says he cannot figure out atm and in the interest of keeping the bots green I'll r+ 
> > this patch in case he wants to land it to investigate things properly.
> 
> Thanks, I will push this now then and will try to figure out what's the true reason and a better solution asap.

I have already attached a patch for bug 72732 that should get testatk.c file back to sanity

Thanks for your understanding!