A11y hierarchy is broken on Ubuntu 8.10 (GNOME 2.24)

RESOLVED FIXED

Status

()

--
blocker
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)

Tracking

({access, fixed1.9.0.11, verified1.9.1})

Trunk
x86
Linux
access, fixed1.9.0.11, verified1.9.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

10 years ago
It was reported at
http://bugzilla.gnome.org/show_bug.cgi?id=535827#c35

The problem is gtk+ loads gail and atk-bridge modules no matter what is set in GTK_MODULES.
So our workaround in bug 312092 is broken.

To completely resolve this issue, we need to make at-spi work with 2 a11y toolkits situation, no matter which module is loaded and initialized firstly.
But currently it's not working, and shutdown atk-bridge and re-init it will cause a crash in libORBit.

So currently we have to use another workaround.
Flags: wanted1.8.1.x?
(Assignee)

Updated

10 years ago
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
(Assignee)

Comment 1

10 years ago
Created attachment 344044 [details] [diff] [review]
patch
(Assignee)

Comment 2

10 years ago
Created attachment 346214 [details] [diff] [review]
patch v2

Use g_object_set instead of gtk_setting_set_string_property as Matthias recommended.
Attachment #344044 - Attachment is obsolete: true
Attachment #346214 - Flags: review?(chpe)
(Assignee)

Updated

10 years ago
Duplicate of this bug: 463383

Comment 4

10 years ago
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

Comment 5

10 years ago
(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.

Comment 6

10 years ago
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.
(Assignee)

Updated

10 years ago
Attachment #346214 - Flags: superreview?(roc)
Attachment #346214 - Flags: review?(chpe)
Attachment #346214 - Flags: review?(aaronleventhal)
Comment on attachment 346214 [details] [diff] [review]
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.
Attachment #346214 - Flags: superreview?(roc) → superreview+

Updated

10 years ago
Attachment #346214 - Flags: review?(aaronleventhal) → review?(surkov.alexander)
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.

Updated

10 years ago
Attachment #346214 - Flags: review?(surkov.alexander) → review+
Thank you, Evan.
(Assignee)

Comment 10

10 years ago
http://hg.mozilla.org/mozilla-central/rev/71c3a9d14712
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Attachment #346214 - Flags: approval1.9.1?

Updated

10 years ago
Depends on: 468845

Comment 11

10 years ago
This apparently caused a crash regression, bug 468845. Please don't land this on the branch without resolving that issue.
Duplicate of this bug: 469496
(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.
(Assignee)

Updated

10 years ago
Attachment #346214 - Flags: approval1.9.1?

Comment 14

10 years ago
Backed out per bug 468845 - http://hg.mozilla.org/mozilla-central/rev/9bc6733d81cc
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 15

10 years ago
Comment on attachment 346214 [details] [diff] [review]
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.
Attachment #346214 - Attachment is obsolete: true
(Assignee)

Comment 16

10 years ago
Created attachment 353657 [details] [diff] [review]
patch v3
Attachment #353657 - Flags: superreview?(roc)
Attachment #353657 - Flags: review?(surkov.alexander)
Comment on attachment 353657 [details] [diff] [review]
patch v3

after some irc discussion with ginn this approach looks right with me, r=me
Attachment #353657 - Flags: review?(surkov.alexander) → review+
Attachment #353657 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 18

10 years ago
Pushed to trunk:
http://hg.mozilla.org/mozilla-central/rev/59508a1ac7dc
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Attachment #353657 - Flags: approval1.9.1?
Comment on attachment 353657 [details] [diff] [review]
patch v3

a191=beltzner
Attachment #353657 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Comment 20

10 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/87f7605e2c5e
Fixed in 1.9.1
Flags: wanted1.8.1.x?
Keywords: fixed1.9.1
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?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ginn, I recommend chatting with Brad Taylor over on #mono-a11y, apparently they are dealing with similar accessibility bootstrapping isues.

Comment 23

10 years ago
(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).

Comment 24

10 years ago
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.
(Assignee)

Comment 25

10 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED

Comment 26

10 years ago
Thanks Ginn. 
Anyone know who to prod at Ubuntu?
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?
(Assignee)

Comment 28

10 years ago
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.

Comment 29

10 years ago
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!
Keywords: fixed1.9.1 → access, verified1.9.1
(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.

Comment 31

10 years ago
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?

Comment 32

10 years ago
Created attachment 358887 [details] [diff] [review]
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.
Comment on attachment 358887 [details] [diff] [review]
Proposed Fix

Evan can you take a look at Brad's patch?
Attachment #358887 - Flags: review?(Evan.Yan)

Comment 34

10 years ago
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?

Comment 35

10 years ago
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).
(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.
Attachment #358887 - Flags: review?(Evan.Yan) → review?(ginn.chen)
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 :)
(Assignee)

Comment 38

10 years ago
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
(Assignee)

Updated

10 years ago
Attachment #358887 - Flags: review?(ginn.chen)

Comment 39

10 years ago
Comment on attachment 358887 [details] [diff] [review]
Proposed Fix

Would be great to have a11y fixed in our ubuntu main browser; requesting approval for 1.9 branch.
Attachment #358887 - Flags: approval1.9.0.9?
Comment on attachment 358887 [details] [diff] [review]
Proposed Fix

Too late for 1.9.0.9, which is code frozen. Will consider for 1.9.0.10.
Attachment #358887 - Flags: approval1.9.0.9? → approval1.9.0.10?

Comment 41

10 years ago
yeah, i actually ment 1.9.0.10. Thanks!
Comment on attachment 358887 [details] [diff] [review]
Proposed Fix

Approved for 1.9.0.10. a=ss
Attachment #358887 - Flags: approval1.9.0.10? → approval1.9.0.10+
(Assignee)

Comment 43

10 years ago
Fix committed to CVS.
Keywords: fixed1.9.0.10
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
I think this is an unlikely suspect. Ginn?
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).
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'.

Comment 48

10 years ago
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.
(Assignee)

Comment 49

10 years ago
The patch has no impact on Windows or GNOME prior to 2.24.2.
(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?
(Assignee)

Updated

9 years ago
Duplicate of this bug: 468239
Ginn, can you provide a status update to the last commenter on https://bugs.launchpad.net/ubuntu/+source/firefox-3.0/+bug/348443 ?
(Assignee)

Comment 53

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

Updated

8 years ago
You need to log in before you can comment on or make changes to this bug.