Firefox at-spi hierarchy is not exposed

Bug #348443 reported by Eitan Isaacson
18
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Critical
firefox-3.0 (Ubuntu)
Fix Released
High
Unassigned

Bug Description

Binary package hint: firefox

To reproduce on Jaunty:
1. Enable accessibility.
2. Log out and back in.
3. Launch Firefox.
4. Launch Accerciser.

Result: The Firefox frame does not have children in Accerciser.

Tags: a11y
Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

Created attachment 344044
patch

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

Created attachment 346214
patch v2

Use g_object_set instead of gtk_setting_set_string_property as Matthias recommended.

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

*** Bug 463383 has been marked as a duplicate of this bug. ***

Revision history for this message
In , SteveLee (steve-fullmeasure) wrote :

Ginn pointed out a work around

In the gconf registry change the value of
/apps/gnome_settings_daemon/gtk-modules/gail:atk-bridge
from
/desktop/gnome/interface/accessibility
to be empty.

Works for me

Revision history for this message
In , WillieWalker (william-walker) wrote :

(In reply to comment #4)
> Ginn pointed out a work around
>
> In the gconf registry change the value of
> /apps/gnome_settings_daemon/gtk-modules/gail:atk-bridge
> from
> /desktop/gnome/interface/accessibility
> to be empty.
>
> Works for me

Keep in mind that this workaround ends up making the logout/shutdown dialog of GNOME 2.24 inaccessible.

Revision history for this message
In , SteveLee (steve-fullmeasure) wrote :

I'm using Fast switcher applet which the upgrade offered to replace the old shutdown and that has a menu of items that work for me.

Revision history for this message
In , Roc-ocallahan (roc-ocallahan) wrote :

Comment on attachment 346214
patch v2

+ nsCString gtkModulesSettingStr(gtkModules);
+ gtkModulesSettingStr.ReplaceSubstring("atk-bridge", "");
+ g_object_set(settings, "gtk-modules", gtkModulesSettingStr.get(), NULL);
+ g_free(gtk_modules_setting);

Fix indent.

Revision history for this message
In , Alexander Surkov (surkov-alexander) wrote :

Ginn, possibly it's better to ask review from Evan because I'm bit far from the code you are fixing? Just it will require some amount of time from me to make the review.

Revision history for this message
In , Alexander Surkov (surkov-alexander) wrote :

Thank you, Evan.

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :
Revision history for this message
In , Benjamin Smedberg (Mozilla) [:bs] (benjamin-smedbergs) wrote :

This apparently caused a crash regression, bug 468845. Please don't land this on the branch without resolving that issue.

Revision history for this message
In , Smichaud (smichaud) wrote :

*** Bug 469496 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Smichaud (smichaud) wrote :

(Following up comment #12)

Oops, my mistake. My bug was (like bug 468845) caused by this patch. So my bug is a dup of bug 468845.

Revision history for this message
In , Benjamin Smedberg (Mozilla) [:bs] (benjamin-smedbergs) wrote :
Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

Comment on attachment 346214
patch v2

This patch have several problems.
Also changing gtk_settings is really dangerous.

So I'd like to take another approach.
We can set an env variable to suppress atk-bridge init at startup with the patch at
http://bugzilla.gnome.org/show_bug.cgi?id=563943
The patch will be in GNOME 2.24.2.

It would be a much safer way to do this.

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

Created attachment 353657
patch v3

Revision history for this message
In , Alexander Surkov (surkov-alexander) wrote :

Comment on attachment 353657
patch v3

after some irc discussion with ginn this approach looks right with me, r=me

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :
Revision history for this message
In , Beltzner (beltzner) wrote :

Comment on attachment 353657
patch v3

a191=beltzner

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :
Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

Hi. Today, on Ubuntu Intrepid I tried FF trunk and couldn't get accessibility info using accerciser. I still required Steve's workaround from comment 4. Can someone confirm my findings?

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

Ginn, I recommend chatting with Brad Taylor over on #mono-a11y, apparently they are dealing with similar accessibility bootstrapping isues.

Revision history for this message
In , SteveLee (steve-fullmeasure) wrote :

(In reply to comment #21)
> Hi. Today, on Ubuntu Intrepid I tried FF trunk and couldn't get accessibility
> info using accerciser. I still required Steve's workaround from comment 4. Can
> someone confirm my findings?

I can confirm that I just rolled back the setting in comment 4 and the problem reappears (using trunk).

Revision history for this message
In , Brad Taylor (brad) wrote :

To fix this issue, disabling AT-SPI (with NO_AT_BRIDGE) is only part of the picture. Let me walk you through what happens when a Gtk+ app loads (e.g.: FF)

   0. ...
   1. gtk_init ()
   2. gtk_init_check ()
   3. gdk_display_open_default_libgtk_only ()
   ...
   4. gtk/gtkmodules.c: display_opened_cb ()
      Searches XSetting for gtk-modules setting set by gnome-settings-daemon
      -> gail is loaded, waits looking for at-spi
      -> atk-bridge is loaded, sees NO_AT_BRIDGE, exits
   5. App inits it's own ATK root window, loads atk-bridge
   6. gail sees atk-bridge, activates, and the app's root window never gets seen.

The fix is basically to also disable gail via an env variable. The upstream gnome bug bgo#565110, and once that's in upstream, you just need to set NO_GAIL when your app starts.

FWIW, we'll be shipping that fix in OpenSUSE 11.1 shortly.

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

David and Steve, the fix requires a fix in atk-bridge. bgo#563943. The fix is in at-spi 1.24.1.
You need to wait for distros update their library.

Brad,
For Firefox, we don't need to suppress the loading of gail.
libgail just sets some callbacks and wait atk-bridge's calling.
When Firefox a11y modules inits, we override these callbacks. So it's not a problem.

Revision history for this message
In , SteveLee (steve-fullmeasure) wrote :

Thanks Ginn.
Anyone know who to prod at Ubuntu?

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

Ginn, thanks for persevering with this painful bug.

At Marco's suggestion I also updated to atk trunk. So with very recent at-spi and atk I tried again. I could not get FF to be accessible without the workaround in comment 4.

Reopen?

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

You don't need to update atk.
Actually the only library required to update is libatk-bridge.so.

You need to make sure the library being loaded is your version not the old version.

On Ubuntu 8.10, you can use this command to config at-spi.
./autogen.sh --prefix=/usr --libexecdir=/usr/lib/at-spi
make && sudo make install

Logout and re-login, it should be fine.

Revision history for this message
In , Marco-zehe (marco-zehe) wrote :

I believe our part of this bug is fixed and stuck on 1.9.1. Ginn, if you think this should not yet be set as verified, please remove the keyword again. Thanks!

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

(In reply to comment #29)
> I believe our part of this bug is fixed and stuck on 1.9.1. Ginn, if you think
> this should not yet be set as verified, please remove the keyword again.
> Thanks!

I'd like to get Brad Taylor's sign off. There might be some wiggly bits left.

Revision history for this message
In , Brad Taylor (brad) wrote :

I have a checkout of 1.9.1 now, and I'm seeing the same behavior as previously. Firefox is only accessible if I add the NO_AT_BRIDGE=1 variable when running. I'll try to hook this up to GDB to get a better understanding of why.

Could someone confirm this for me?

Revision history for this message
In , Brad Taylor (brad) wrote :

Created attachment 358887
Proposed Fix

Doing some digging, I realized that with Ginn's fix NO_AT_BRIDGE=1 is only set when GTK_MODULES is set. With bgo#535827 and friends, the GTK_MODULES env var has been obsoleted by an XSetting.

The attached patch moves the PR_SetEnv("NO_AT_BRIDGE=1") outside of the GTK_MODULES check so that it's set regardless.

The only downside that I can see with this change is that the NO_AT_BRIDGE env var will be set to 1 and then unset even when a11y isn't enabled. If this is a problem, Firefox will need a better way of detecting whether a11y is enabled than checking the GTK_MODULES env var. Looking at the /desktop/gnome/interface/accessibility GConf key might be a good way to go.

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

Comment on attachment 358887
Proposed Fix

Evan can you take a look at Brad's patch?

Revision history for this message
In , Alexander Sack (asac) wrote :

disabling at-bridge will make most a11y desktop tools not work, right? Can someone clarify, whether this is the case and if so why this is something we consider?

Revision history for this message
In , Brad Taylor (brad) wrote :

Alexander -- that is most certainly not what is proposed or happening.

The NO_AT_BRIDGE env variable is used _temporarily_ in the process-local environment by applications which need to inhibit atk-bridge's startup occuring due to initializing GTK+. While I'm not clear on the specifics behind Firefox's need for this, this is most commonly done because application level a11y support hasn't been loaded yet. Once the app is ready, NO_AT_BRIDGE is set to 0, and then Firefox and other applications will manually load the atk-bridge module.

Again, this is process-local, and only done for a short amount of time so as to not affect processes invoked by Firefox (which I'm assuming will share the environment).

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

(In reply to comment #35)

Right.

> The NO_AT_BRIDGE env variable is used _temporarily_ in the process-local
> environment by applications which need to inhibit atk-bridge's startup occuring
> due to initializing GTK+. While I'm not clear on the specifics behind
> Firefox's need for this, this is most commonly done because application level
> a11y support hasn't been loaded yet.

I believe this is done so that we can do lazy instantiation of the added accessibility processing. While I believe FF to be the most accessible browser, we have to be cognizant that browser choice is often made on performance criteria. In this way, we actually stand a better chance of spreading accessibility if we only turn it on when needed.

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

Don't believe me yet though, I came into this late and I'm not sure of the reasons. When Ginn's back from vacation we should get the real deal :)

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

Thank you, Brad!

I just realized the problem was when I land the patch to 1.9.1, I made this mistake.
Thanks for pointing out.

mozilla-central is fine.

So I think, technically, think we just need to correct the landing.

I just did it.
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7b0ad3180b30

Revision history for this message
Eitan Isaacson (eeejay) wrote :

Binary package hint: firefox

To reproduce on Jaunty:
1. Enable accessibility.
2. Log out and back in.
3. Launch Firefox.
4. Launch Accerciser.

Result: The Firefox frame does not have children in Accerciser.

Revision history for this message
Eitan Isaacson (eeejay) wrote : apport-collect data

Architecture: i386
DistroRelease: Ubuntu 9.04
Package: firefox 3.0.7+nobinonly-0ubuntu1
PackageArchitecture: all
ProcEnviron:
 SHELL=/bin/bash
 LANG=en_US.UTF-8
Uname: Linux 2.6.28-11-generic i686
UserGroups: adm admin cdrom dialout lpadmin plugdev pulse-access pulse-rt sambashare

Revision history for this message
Eitan Isaacson (eeejay) wrote :
Revision history for this message
Javier Collado (javier.collado) wrote :

davidb at irc.mozilla.org kindly pointed me to a workaround that solves the
problem (comment #4):
https://bugzilla.mozilla.org/show_bug.cgi?id=460926#c4

Now it's possible to write automated testing scripts for Firefox 3 in Ubuntu.

Revision history for this message
Ara Pulido (ara) wrote :

The bug is already known upstream, and apparently it has been fixed in trunk.

Changed in firefox:
importance: Undecided → High
status: New → Triaged
Revision history for this message
Ara Pulido (ara) wrote :

Although the workaround works OK for testing, we should advocate for the real fix to land in Ubuntu.

Revision history for this message
Javier Collado (javier.collado) wrote :

I've been told the problem in the bug is solved, but the new way of instantiating ally has not been yet adopted by the distro. I'm not an expert, but it seems we won't need the workaround after this is updated.

Revision history for this message
Javier Collado (javier.collado) wrote :

Ginn Chenn from the Sun team working on Mozilla Accessibility (http://www.mozilla.org/access/unix/architecture) told me that apart from the workaround commented before, the issue should be fixed with at-spi 1.24.1. The version in my current installation is 1.24.0, but as soon I get an update I'll let you know if it works for me.

Revision history for this message
Javier Collado (javier.collado) wrote :

Latest news I've received is that the fix does not only depend on at-spi 1.24.1, but also on firefox 3.1 beta 3 or later to work correctly.

Revision history for this message
Ara Pulido (ara) wrote :

at-spi in Jaunty is already 1.26:

ara@sushirider:~$ apt-cache policy at-spi
at-spi:
  Installed: 1.26.0-0ubuntu2
  Candidate: 1.26.0-0ubuntu2
  Version table:
 *** 1.26.0-0ubuntu2 0
        500 http://archive.ubuntu.com jaunty/main Packages
        100 /var/lib/dpkg/status

Alexander (asac), is there any possibilities to cherry-pick the patch into our package?

Revision history for this message
Ara Pulido (ara) wrote :

I meant, the patch in firefox

Changed in firefox:
status: Unknown → Fix Released
Revision history for this message
In , Alexander Sack (asac) wrote :

Comment on attachment 358887
Proposed Fix

Would be great to have a11y fixed in our ubuntu main browser; requesting approval for 1.9 branch.

Revision history for this message
In , Samuel-sidler+old (samuel-sidler+old) wrote :

Comment on attachment 358887
Proposed Fix

Too late for 1.9.0.9, which is code frozen. Will consider for 1.9.0.10.

Revision history for this message
In , Alexander Sack (asac) wrote :

yeah, i actually ment 1.9.0.10. Thanks!

Revision history for this message
In , Samuel-sidler+old (samuel-sidler+old) wrote :

Comment on attachment 358887
Proposed Fix

Approved for 1.9.0.10. a=ss

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

Fix committed to CVS.

Revision history for this message
In , Daniel Holbert (dholbert) wrote :

Four Windows unit-test cycles have gone by since this landed, and three appear to have had sporadic timeouts (probably unrelated this bug) before they got to any a11y tests.

However, in the one cycle that *didn't* timeout, we actually failed some a11y tests, which I'm guessing might be related to this bug's landing. The failures were:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.0/1239715197.1239718368.18523.gz
*** 135 ERROR FAIL | Wrong startIndex value for ID emptyLink! | got 97, expected 98 | chrome://mochikit/content/a11y/accessible/test_nsIAccessibleHyperLink.html
*** 136 ERROR FAIL | Wrong endIndex value for ID emptyLink! | got 98, expected 99 | chrome://mochikit/content/a11y/accessible/test_nsIAccessibleHyperLink.html
*** 143 ERROR FAIL | Wrong startIndex value for ID LinkWithSpan! | got 123, expected 124 | chrome://mochikit/content/a11y/accessible/test_nsIAccessibleHyperLink.html
*** 144 ERROR FAIL | Wrong endIndex value for ID LinkWithSpan! | got 124, expected 125 | chrome://mochikit/content/a11y/accessible/test_nsIAccessibleHyperLink.html
*** 156 ERROR FAIL | Wrong startIndex value for ID namedAnchor! | got 201, expected 202 | chrome://mochikit/content/a11y/accessible/test_nsIAccessibleHyperLink.html
*** 157 ERROR FAIL | Wrong endIndex value for ID namedAnchor! | got 202, expected 203 | chrome://mochikit/content/a11y/accessible/test_nsIAccessibleHyperLink.html

URL of failing test:
http://mxr.mozilla.org/seamonkey/source/accessible/tests/mochitest/test_nsIAccessibleHyperLink.html

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

I think this is an unlikely suspect. Ginn?

Revision history for this message
In , Daniel Holbert (dholbert) wrote :

Yeah, it's just that it was the *only* possible patch that could be suspect, since the failure was new failure (it hadn't happened in the 12 hours before this change landed), and it's accessibility-related which is a bit suspicious.

However, during the time since I posted comment 44, another unittest cycle has completed, and it was green:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.0/1239715197.1239730052.17163.gz

So maybe this was just a sporadic failure. I don't particularly trust these particular tinderboxes -- they've been sporadically red a lot recently (I just filed bug 488345 on that).

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

Cool and thanks Daniel, for the original comment and the followup. I'd rather hear about these things than not... it is the good kinda 'noise'.

Revision history for this message
In , Marco-zehe (marco-zehe) wrote :

I know where these failures come from, and they are not related to this bug. David, this is the bug where we load an external image map, which we fixed on m-c not too long ago.

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

The patch has no impact on Windows or GNOME prior to 2.24.2.

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

(In reply to comment #48)
> I know where these failures come from, and they are not related to this bug.
> David, this is the bug where we load an external image map, which we fixed on
> m-c not too long ago.

Ah right! Did the fix not propogate?

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

*** Bug 468239 has been marked as a duplicate of this bug. ***

Revision history for this message
Javier Collado (javier.collado) wrote :

After some time, I've checked if this is working now, but it still doesn't with firefox 3.0.10+nobinonly-0ubuntu0.9.04.1

Does anybody have any update about this problem?

Revision history for this message
In , Bolterbugz (bolterbugz) wrote :

Ginn, can you provide a status update to the last commenter on https://bugs.launchpad.net/ubuntu/+source/firefox-3.0/+bug/348443 ?

Revision history for this message
Ginn (ginnchen) wrote :

Javier,

The patch was committed to Firefox 3.0.11 not 3.0.10.

Revision history for this message
In , Ginn-chen (ginn-chen) wrote :

I've commented there. The fix is in 3.0.11 not 3.0.10.

Revision history for this message
Micah Gersten (micahg) wrote :

Fix should be in 3.0.11, 3.0.12 is already out. Let us know if you're still having issues.

Changed in firefox-3.0 (Ubuntu):
status: Triaged → Fix Released
Changed in firefox:
importance: Unknown → Critical
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.