957254 - Accessible window events missing for javascript alerts

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Joanmarie Diggs]

Attachment: testcase: alert.html

Steps to reproduce:
1. View the attached test case in Firefox
2. Load the attached accessible-event listener in a terminal
3. Press the "Press me" button

Expected results: A window:activate event would appear for the resulting alert dialog.

Actual results: There is no window:activate event for the resulting alert dialog.

Comment 1

[Joanmarie Diggs]

Attachment: accessible-event listener: window.py

Comment 2

[alexander :surkov (:asurkov)]

alert as it's implemented is not typical window so no events. Joanie, do you want activate events for ARIA dialogs? Should deactivate event be fired? Does it make sense for modal dialogs?

Comment 3

[Joanmarie Diggs]

I'd like alerts and dialogs to be consistent with alerts and dialogs across the board. Otherwise, there's far too much heuristic custom handling on a per-implementation basis.

* In Firefox, real dialogs (e.g. Preferences) result in window: events
* In Epiphany and Chrome, in addition to the above, the test case attached to this bug result in window: events.
* In Gtk and other desktop-app toolkits, dialogs and alerts result in window: events.

Long way of saying: "Please", "yes", and "I think it does" respectively to your questions. :)

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Joanmarie Diggs from comment #3)
> I'd like alerts and dialogs to be consistent with alerts and dialogs across
> the board. Otherwise, there's far too much heuristic custom handling on a
> per-implementation basis.

sure

> Long way of saying: "Please", "yes", and "I think it does" respectively to
> your questions. :)

so if modal dialog is getting closed then you would expect to see deactivate event before it's closed, is that correct?

Comment 5

[Joanmarie Diggs]

(In reply to alexander :surkov from comment #4)
> (In reply to Joanmarie Diggs from comment #3)
> > I'd like alerts and dialogs to be consistent with alerts and dialogs across
> > the board. Otherwise, there's far too much heuristic custom handling on a
> > per-implementation basis.
> 
> sure

\o/ I owe you a beer. :)
 
> so if modal dialog is getting closed then you would expect to see deactivate
> event before it's closed, is that correct?

Yes. And this deactivate event should precede the activate event for the app window that is about to become re-active as a result of the modal dialog going away.

Comment 6

[Jonathan Wei [:jwei]]

Attachment: Added window:activate/deactivate events for dialogs

Window activate and deactivate events are fired when elements with ARIA role = dialog are shown and hidden.  This includes JavaScript alerts.

Comment 7

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8368717 [details] [diff] [review]
Added window:activate/deactivate events for dialogs

>+        if (!accessible->IsRoot() && accessible->HasARIARole() &&
>+          accessible->ARIARole() == roles::DIALOG) {

just use accessible->Role()

>+          guint id = g_signal_lookup ("deactivate", MAI_TYPE_ATK_OBJECT);

no space before (

>     case nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE:
>         g_signal_emit_by_name (atkObj, "load_complete");
>+        if (!accessible->IsRoot() && accessible->HasARIARole() &&
>+            accessible->ARIARole() == roles::DIALOG) {
>+          guint id = g_signal_lookup ("activate", MAI_TYPE_ATK_OBJECT);
>+          g_signal_emit(atkObj, id, 0);

same on both counts here

Comment 8

[Jonathan Wei [:jwei]]

Attachment: Added window:activate/deactivate events for dialogs v2

Comment 9

[Jonathan Wei [:jwei]]

When accessible->Role() is used instead of accessible->ARIARole(), an assertion is triggered in the treeupdate/test_listbox.xul test.  This is when the list item is removed, since we check if accessible->Role() == roles::DIALOG for all hide events.  The item is removed through the DOM, so by the time we call NativeRole() on the accessible, GetListAccessible() returns null, triggering the assertion.

Should accessible->Role() checks be kept, and add an assertion flag for the mochitest (and trigger an assertion any time a XUL listitem is removed in a debug build on Linux with accessibility enabled), or keep the ARIARole() check?

Comment 10

[Trevor Saunders (:tbsaunde)]

(In reply to Jonathan Wei [:jwei] from comment #9)
> When accessible->Role() is used instead of accessible->ARIARole(), an
> assertion is triggered in the treeupdate/test_listbox.xul test.  This is
> when the list item is removed, since we check if accessible->Role() ==
> roles::DIALOG for all hide events.  The item is removed through the DOM, so
> by the time we call NativeRole() on the accessible, GetListAccessible()
> returns null, triggering the assertion.

ugh, leave it and add comment like
// XXX handle native dialog accessibles
or something like that.

Comment 11

[Jonathan Wei [:jwei]]

Attachment: Added window:activate/deactivate events for dialogs v3

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b87ae646c62

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/4b87ae646c62