350425 - Ensure a11y support in breakpad

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Ginn Chen]

At-poke can't find talkback client, and the client doesn't emit any a11y events, such as "focus".

Disabled person can not close the talkback window when it pops up.

Comment 1

[Aaron Leventhal]

Our QA has found this to be a bug on Windows as well.

Regression or not? Has the talkback client changed since 1.5?

Comment 2

[Aaron Leventhal]

> Our QA has found this to be a bug on Windows as well.
I should say, our QA tested and found that on Windows, the Talkback client is not accessible with a screen reader.

Comment 3

[Wayne DeAngelo]

Update
env: Bon echo nightly build 08302006, either JAWS or Window Eyes.

Steps to recreate:

1. Start screen reader & bon echo.
2. Use url http://toadstool.se/software/iexploder/demo/iexploder.cgi?lookup=1&test=243244&subtest
3. This URL will create a Firefox crash.
3. Note, the dialog window that stat that firefox.exe has encountered a problem and needs to close, .... is not anounced by the screenreader.

Comment 4

[Aaron Leventhal]

I'm seeing native controls and correct objects/focus events when I look with Inspect and Spy++. This may not be a Firefox bug at all, at least on the Windows side. Will keep testing.

Comment 5

[Aaron Leventhal]

Clearing blocking request. I am not seeing any Firefox bug on Windows. Need more information from Wayne about what he is getting before we request blocking.

Comment 6

[Ginn Chen]

I think current talkback client is a X-app not a GTK2 app.
Does bug 216827 solve this issue?

Comment 7

[Aaron Leventhal]

I would say it does.

Comment 8

[Wayne DeAngelo]

Opened JAWS & WE defects as requested by Aaron L. see WE bug 3390

Comment 9

[Nian Liu(n/a in a long time)]

agree with ginn that talkback is x app. not sure how to make it accessible

[neoliu@dizzy talkback]$ ldd talkback.so
        linux-gate.so.1 =>  (0x00ab9000)
        libstdc++.so.5 => /usr/lib/libstdc++.so.5 (0x00111000)
        libm.so.6 => /lib/libm.so.6 (0x001cb000)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x009c9000)
        libc.so.6 => /lib/libc.so.6 (0x00da1000)
        /lib/ld-linux.so.2 (0x00779000)

Comment 10

[Håkan Waara]

Note that in the long term, I think word is that Talkback is being replaced by a new cross-platform open-source project named Airbag. 

It just started though, so it's not even near complete... 

http://code.google.com/p/airbag/

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Yeah, it seems that this bug either needs to be morphed or be resolved invalid now that Breakpad will be taking over very soon on the trunk.

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

If this bug is really about Mozilla's Breakpad client(s), then it belongs in Toolkit:Breakpad.

Comment 13

[(not currently active) Ted Mielczarek]

We don't even have a Linux breakpad client yet...

Comment 14

[Marco Zehe (:MarcoZ)]

This still does not work on Linux. I just had a crash in Thunderbird trunk on Linux, and Orca told me that the Crash Reporter is inaccessible. So if the switch to Breakpad has occurred on Linux yet, IT IS NOT WORKING YET!

Comment 15

[(not currently active) Ted Mielczarek]

We do in fact have a Linux Breakpad client now.  It uses GTK2, and we'd like to make it accessible, but I don't really know anything about a11y, so if someone would like to make this happen, the crash reporter client code is here:
http://mxr.mozilla.org/mozilla/source/toolkit/crashreporter/client/
with the Linux-specific parts here:
http://mxr.mozilla.org/mozilla/source/toolkit/crashreporter/client/crashreporter_linux.cpp

Comment 16

[Aaron Leventhal]

Ginn, can you tell Ted how to turn on Gnome a11y support for the Linux Breakpad client -- it already uses GTK2.

Comment 17

[Ginn Chen]

If gnome_program_init() is called at startup, it will have proper a11y functionality.

Comment 18

[(not currently active) Ted Mielczarek]

Attachment: call gnome_program_init in crashreporter

This patch dynamically loads libgnome and calls gnome_program_init.  I borrowed this code from nsNativeAppSupportUnix.cpp.  I tested this by running Accerciser (http://live.gnome.org/Accerciser) and noting that it could see the widgets in the crashreporter app.  I don't know what the expected results are, so we still may have less than optimal a11y in the app, but this should get us started, at least.

Comment 19

[Evan Yan]

Comment on attachment 278591 [details] [diff] [review]
call gnome_program_init in crashreporter

>+static void TryInitGnome()
>+{
>+  void* gnomeLib = dlopen("libgnome-2.so.0", RTLD_LAZY);
>+  if (!gnomeLib)
>+    return;
>+  void* gnomeuiLib = dlopen("libgnomeui-2.so.0", RTLD_LAZY);
>+  if (!gnomeuiLib)
>+    return;

I guess we need to dlclose(gnomeLib) here before return.

And if dlopen failed, would it be better to print a error message here to tell user accessibility and/or some other things can't be enabled?

Comment 20

[Dave Camp (:dcamp)]

Comment on attachment 278591 [details] [diff] [review]
call gnome_program_init in crashreporter


>+  void* gnomeuiLib = dlopen("libgnomeui-2.so.0", RTLD_LAZY);
>+  if (!gnomeuiLib)
>+    return;
>+
>+  _gnome_program_init_fn gnome_program_init =
>+    reinterpret_cast<_gnome_program_init_fn>(dlsym(gnomeLib, "gnome_program_init"));
>+  _libgnomeui_module_info_get_fn libgnomeui_module_info_get =
>+    reinterpret_cast<_libgnomeui_module_info_get_fn>(dlsym(gnomeuiLib, "libgnomeui_module_info_get"));
>+
>+  if (gnome_program_init && libgnomeui_module_info_get) {
>+    gnome_program_init("crashreporter", "1.0", libgnomeui_module_info_get(),
>+                       gArgc, gArgv, NULL);
>+  }

I don't think you need libgnomeui to be initialized for the accessibility modules to be loaded, you might be able to get rid of that stuff.

>+  dlclose(gnomeLib);
>+  dlclose(gnomeuiLib);

I don't think the module should be unloaded before shutdown...

Comment 21

[Dave Camp (:dcamp)]

Comment on attachment 278591 [details] [diff] [review]
call gnome_program_init in crashreporter


>+  _gnome_program_init_fn gnome_program_init =
>+    reinterpret_cast<_gnome_program_init_fn>(dlsym(gnomeLib, "gnome_program_init"));
>+  _libgnomeui_module_info_get_fn libgnomeui_module_info_get =
>+    reinterpret_cast<_libgnomeui_module_info_get_fn>(dlsym(gnomeuiLib, "libgnomeui_module_info_get"));
>+  if (gnome_program_init && libgnomeui_module_info_get) {
>+    gnome_program_init("crashreporter", "1.0", libgnomeui_module_info_get(),
>+                       gArgc, gArgv, NULL);
>+  }

You should be able to use libgnome_module_info_get from libgnome as the third arg to gnome_program_init()

Comment 22

[(not currently active) Ted Mielczarek]

Attachment: call gnome_program_init in crashreporter + some cleanup [checked in]

So, it turns out you do in fact need to call libgnomeui_module_info_get there to make a11y work.  I tried it with libgnome_module_info_get and it doesn't work.  This patch moves the dlclose calls to UIShutdown, but otherwise is the same.

(In reply to comment #19)
> And if dlopen failed, would it be better to print a error message here to tell
> user accessibility and/or some other things can't be enabled?

I don't think so.  To have a11y at all, the user will have to have Gnome installed, so they probably won't care that a11y doesn't work.  Also, we're not guaranteed to have a console anyway.

Comment 23

[Dave Camp (:dcamp)]

Comment on attachment 278954 [details] [diff] [review]
call gnome_program_init in crashreporter + some cleanup [checked in]

that's too bad..

Comment 24

[(not currently active) Ted Mielczarek]

Comment on attachment 278954 [details] [diff] [review]
call gnome_program_init in crashreporter + some cleanup [checked in]

Checked in.  Leaving this open for followup on just how our app fares in a11y tools.  :)  I've verified that Accerciser can see the app and the widgets it contains, but I don't know anything about a11y practices, so someone more qualified will have to check it out and let me know if we're doing anything that doesn't work well with a11y tools.

Comment 25

[Boris Zbarsky [:bzbarsky]]

This doesn't compile over here:

../../../../mozilla/toolkit/crashreporter/client/crashreporter_linux.cpp: In 
   function `void TryInitGnome()':
../../../../mozilla/toolkit/crashreporter/client/crashreporter_linux.cpp:301: error: ISO
   C++ forbids casting between pointer-to-function and pointer-to-object
../../../../mozilla/toolkit/crashreporter/client/crashreporter_linux.cpp:303: error: ISO
   C++ forbids casting between pointer-to-function and pointer-to-object
gmake[6]: *** [crashreporter_linux.o] Error 1

Comment 26

[(not currently active) Ted Mielczarek]

Do you have warnings-as-errors or something set, or are you using a newer GCC?  Also, does it compile if you change to C-style casts?

Comment 27

[Boris Zbarsky [:bzbarsky]]

> Do you have warnings-as-errors or something set,

Not unless it's set by default somehow.  So probably not.

> are you using a newer GCC?

Using g++ 3.3.4.2

> Also, does it compile if you change to C-style casts?

Yes.  Some googling gives http://www.trilithium.com/johan/2004/12/problem-with-dlsym/

Comment 28

[(not currently active) Ted Mielczarek]

Ah, gcc 3.3?  Interesting.  And yes, I read the link, but it compiled ok for me and on the tinderbox, so I thought I was safe.  (Although it did lead me to file bug 394311).  If a C-style cast fixes it for you, then r=me to change the two casts.

Comment 29

[Boris Zbarsky [:bzbarsky]]

I did that, but frankly, I would have expected you to fix up your issues yourself instead of making someone else have to stay up and watch the tree....

Comment 30

[Marco Zehe (:MarcoZ)]

This is now working with Orca and Accerciser. Closing bug.

Comment 31

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381