The default bug view has changed. See this FAQ.

[atk] Page setup dialog is not added to Firefox a11y tree

RESOLVED FIXED in mozilla9

Status

()

Core
Disability Access APIs
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ginn Chen, Assigned: Ginn Chen)

Tracking

unspecified
mozilla9
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
On Linux, enable a11y, start Page Start dialog from File->Page Setup ...

You cannot find the dialog in Firefox a11y tree in accerciser.

The problem is we're using gtk_print_run_page_setup_dialog().
So we don't have a chance to do the trick of RunDialog().

This is a minor issue because
1) The dialog still works with a11y tools, e.g. Orca.
2) Page Setup can be accessed from "Print ..." dialog, page setup tab.


I hope we can have a generic fix for the gtk dialogs rather than re-implement gtk_print_run_page_setup_dialog() in our code.
(Assignee)

Comment 1

6 years ago
Perhaps we can do something like gail_toplevel_show_event_watcher() / gail_toplevel_hide_event_watcher().
(Assignee)

Comment 2

6 years ago
Created attachment 543893 [details] [diff] [review]
patch

I think it is a much better solution.
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #543893 - Flags: review?(surkov.alexander)

Comment 3

6 years ago
Comment on attachment 543893 [details] [diff] [review]
patch

please fix style:
1) type* obj; vs type *obj;
2) use explicit casting

>+
>+        quark_gecko_acc_obj = g_quark_from_static_string("GeckoAccObj");
>+        g_signal_add_emission_hook(g_signal_lookup("show", GTK_TYPE_WINDOW),
>+          0, toplevel_event_watcher, (gpointer)nsIAccessibleEvent::EVENT_SHOW, (GDestroyNotify) NULL);
>+        g_signal_add_emission_hook(g_signal_lookup("hide", GTK_TYPE_WINDOW),
>+          0, toplevel_event_watcher, (gpointer)nsIAccessibleEvent::EVENT_HIDE, (GDestroyNotify) NULL);

should you pass quark_gecko_acc_obj as second argument or what quark_gecko_acc_obj is for?
should you remove emission hook when a11y shutdowns?
does it trigger for gecko dialogs only and if yes then how does work?
(Assignee)

Comment 4

6 years ago
Created attachment 544170 [details] [diff] [review]
patch v2

comments addressed and some small changes in nsApplicationAccessibleWrap.cpp
Attachment #543893 - Attachment is obsolete: true
Attachment #544170 - Flags: review?(surkov.alexander)
Attachment #543893 - Flags: review?(surkov.alexander)
(Assignee)

Comment 5

6 years ago
(In reply to comment #3)
> should you pass quark_gecko_acc_obj as second argument or what
> quark_gecko_acc_obj is for?

quark_gecko_acc_obj is static in this file, so I don't need to pass it.
It is an uniquely identify for the string "GeckoAccObj".
I need a GQuark to save according GeckoAccObj pointer into AtkObject.

> should you remove emission hook when a11y shutdowns?

I don't think atk-bridge/gail/... can really be shutdown/init at runtime.
So unless we actually support this feature, I'll leave the hooks.
We can use quark_gecko_acc_obj to avoid adding hooks again.

gail also leaves hooks in their code.

> does it trigger for gecko dialogs only and if yes then how does work?

The hook is triggered for all GTK window show/hide event, e.g. Mozilla window/dialog, menu window, combobox dropdown, tooltip, GTK dialog, etc.

(!IS_MAI_OBJECT(child) && (atk_object_get_role(child) == ATK_ROLE_DIALOG))
makes sure we only operate on GTK native dialogs, e.g. page setup, file chooser, print dialog, etc.

Comment 6

6 years ago
(In reply to comment #5)
> (In reply to comment #3)

> > should you remove emission hook when a11y shutdowns?
> 
> I don't think atk-bridge/gail/... can really be shutdown/init at runtime.

I don't get answer, sorry. I meant if there's a function that adds a hook and there's a function to remove the hook then why we don't remove it when we don't need it anymore, for example, when Firefox is closed. Is it removed automatically when Firefox closes or hangs until you reboot the system?

> So unless we actually support this feature, I'll leave the hooks.
> We can use quark_gecko_acc_obj to avoid adding hooks again.
> 
> gail also leaves hooks in their code.
> 
> > does it trigger for gecko dialogs only and if yes then how does work?
> 
> The hook is triggered for all GTK window show/hide event, e.g. Mozilla
> window/dialog, menu window, combobox dropdown, tooltip, GTK dialog, etc.
> 
> (!IS_MAI_OBJECT(child) && (atk_object_get_role(child) == ATK_ROLE_DIALOG))
> makes sure we only operate on GTK native dialogs, e.g. page setup, file
> chooser, print dialog, etc.

That's not what I meant to ask, sorry. Why are only native windows open by Firefox triggers the installed hook? Is this hook local and we are not notified by native windows open by other applications?

Comment 7

6 years ago
Comment on attachment 544170 [details] [diff] [review]
patch v2

r=me for gecko part, I'd like if someone will look at gail calls. Trevor, can you be this person?

Also I think we'd need r+ from Robert since we invade his area.
Attachment #544170 - Flags: review?(trev.saunders)
Attachment #544170 - Flags: review?(surkov.alexander)
Attachment #544170 - Flags: review?(roc)
Attachment #544170 - Flags: review+
Comment on attachment 544170 [details] [diff] [review]
patch v2

>+
+        if (!quark_gecko_acc_obj) {
>+          quark_gecko_acc_obj = g_quark_from_static_string("GeckoAccObj");
gobject doesn't let us do that statically? :(

>+          g_signal_add_emission_hook(g_signal_lookup("show", GTK_TYPE_WINDOW),
>+            0, toplevel_event_watcher,
>+            reinterpret_cast<gpointer>(nsIAccessibleEvent::EVENT_SHOW), NULL);
>+          g_signal_add_emission_hook(g_signal_lookup("hide", GTK_TYPE_WINDOW),
>+            0, toplevel_event_watcher,
>+            reinterpret_cast<gpointer>(nsIAccessibleEvent::EVENT_HIDE), NULL);

I think we really should remove these on shutdown.  Its not hard, and even if we can't turn accessibility all the way off yet, we can atleast make the performance impact less.
Attachment #544170 - Flags: review?(trev.saunders)
Attachment #544170 - Flags: review+
> I don't get answer, sorry. I meant if there's a function that adds a hook
> and there's a function to remove the hook then why we don't remove it when
> we don't need it anymore, for example, when Firefox is closed. Is it removed
> automatically when Firefox closes or hangs until you reboot the system?

its local to our process, so it necessarily goes away when our process goes away.

> > gail also leaves hooks in their code.

that doesn't make it a good thing to leave more.

> That's not what I meant to ask, sorry. Why are only native windows open by
> Firefox triggers the installed hook? Is this hook local and we are not
> notified by native windows open by other applications?

yes, its only for windows in our process
(Assignee)

Comment 10

6 years ago
Created attachment 545087 [details] [diff] [review]
patch v3

1) static GQuark sQuark_gecko_acc_obj = g_quark_from_static_string("GeckoAccObj");
2) Remove hook in nsApplicationAccessibleWrap::Unload()
3) Add s- prefix for all static variables in this file.
4) Remove trailing whitespaces.
Attachment #545087 - Flags: review?(trev.saunders)
Comment on attachment 545087 [details] [diff] [review]
patch v3

> 
>-static GHashTable *listener_list = NULL;
>-static gint listener_idx = 1;
>+static GHashTable *sListener_list = NULL;

type* forAlex

>+static GHashTable *sKey_listener_list = NULL;

same

>+static PRBool sToplevel_event_hook_added = PR_FALSE;

just use bool and true / false

r=me thanks
Attachment #545087 - Flags: review?(trev.saunders)
Attachment #545087 - Flags: review?(roc)
Attachment #545087 - Flags: review+
Attachment #544170 - Flags: review?(roc)
Comment on attachment 545087 [details] [diff] [review]
patch v3

Review of attachment 545087 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #545087 - Flags: review?(roc) → review+
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/mozilla-central/rev/34b0b3bc6984
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8

Comment 14

6 years ago
When I start firefox I get the message

(process:16712): GLib-CRITICAL **: g_slice_set_config: assertion `sys_page_size == 0' failed

on the console.

The first bad revision is:
changeset:   72769:34b0b3bc6984
user:        Ginn Chen <ginn.chen@oracle.com>
date:        Thu Jul 14 09:58:32 2011 +0800
summary:     Bug 669130 [atk]Use emission hook for show/hide signal to add/remove GTK+ native a11y dialog to children of Mozilla application accessible. r=trev.saunders,roc

Comment 15

6 years ago
This happens because sQuark_gecko_acc_obj is static and therefore call to g_quark_from_static_string and then g_slice init part is made as soon as libxul.so is dlopened.

But when g_slice_set_config is called inside XRE_Main, to disable, g_slice allocator, g_slice refuses because it has already been initialized.

So, the result is an regression of bug #431221

Updated

6 years ago
Blocks: 431221
(Assignee)

Comment 16

6 years ago
Created attachment 546971 [details] [diff] [review]
patch

Do not init sQuark_gecko_acc_obj staticly.
Attachment #546971 - Flags: review?(trev.saunders)
(Assignee)

Comment 17

6 years ago
Comment on attachment 546971 [details] [diff] [review]
patch

oops, there's already a patch in Bug 672671.
Attachment #546971 - Flags: review?(trev.saunders)
(Assignee)

Updated

6 years ago
Depends on: 672671

Comment 18

6 years ago
(In reply to Ginn Chen from comment #13)
> http://hg.mozilla.org/mozilla-central/rev/34b0b3bc6984

Ginn, it appears you forgot to remove nsAccessibilityHelper.h/cpp files
(Assignee)

Comment 19

6 years ago
(In reply to alexander surkov from comment #18)
> (In reply to Ginn Chen from comment #13)
> > http://hg.mozilla.org/mozilla-central/rev/34b0b3bc6984
> 
> Ginn, it appears you forgot to remove nsAccessibilityHelper.h/cpp files

Thanks!

Now I removed them.
http://hg.mozilla.org/integration/mozilla-inbound/rev/1a11c6516cda
https://hg.mozilla.org/mozilla-central/rev/1a11c6516cda
Target Milestone: mozilla8 → mozilla9
You need to log in before you can comment on or make changes to this bug.