343136 - Support ATK document events

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

We need to support events fired by Document instances, which include:

    * "document:load-complete" A pending (static) document content load has completed.
    * "document:reload" A reload of the document content has been initiated.
    * "document:load-stopped" Loading of the document content has been interrupted.
    * "document:content-changed" The contents of the Document container has changed.
    * "document:attributes-changed" The global attributes (i.e. document-wide attributes) of the Document have changed.

Comment 1

[parente]

I'm wondering what payload should be delivered with these events. One thing that might start the discussion is for me to ask, what other non-document events does Firefox fire when a page is loaded/reloaded? Does it now fire a focus event on the document pane when page loading is complete? If so, will it continue to do this once the document events are implemented.

Comment 2

[Aaron Leventhal]

We currently fire EVENT_REORDER on the document when the loading is complete. There's not necessarily a focus event on the document itself because the user may go back or forward in history, in which case they will be focused on what they were focused on before. In addition, if the user moves to the location or search bar while the document is loading, we don't reset the focus to the document when it finally loads, because we don't want to interrupt their typing.

Comment 3

[Aaron Leventhal]

And yes, I believe it makes sense for us to still fire EVENT_REORDER on the document when a new page gets loaded.

Comment 4

[parente]

> And yes, I believe it makes sense for us to still fire EVENT_REORDER on the
> document when a new page gets loaded.

What is the EVENT_REORDER equivalent in atk/at-spi?

For each of the document events, one would expect the accessible implementing the Document interface to be delivered as the event source. That leaves two integer fields and one variant data field to fill with information if need be. Would you want to deliver any other information? For load-complete and reload, it might be nice to have the URL for the document as the variant. For attributes-changed, a list of the attribute names on the document object that changed might also help. Is this possible? What about the others?

Comment 5

[Aaron Leventhal]

I believe EVENT_REORDER maps to object:children-changed

As far as what to do with detail1 and detail2 we'd like to do what all the other apps do. Sounds like a question for the gnome-accessibility-devel list to me.

Comment 6

[parente]

No one else fires these events yet AFAIK. Worth checking with the OpenOffice and Adobe people in case I'm wrong. My guess is that whatever Firefox does will set the standard that all others will (hopefully) mimick.

Comment 7

[parente]

This topic has been proposed for the GNOME a11y summit. Not sure if it will get discussed, but the information needs to be documented somewhere, perhaps at http://accessibility.freestandards.org/a11yspecs/atspi/adoc/atspi-events.html for the time being and later in the atk/gtk guide.

Comment 8

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

for load-complete
biesi notes that STATE_STOP alone does probably not suffice
biesi since that flag is also set just when an image load or something stops

Comment 9

[Aaron Leventhal]

Just hook into  nsDocAccessibleWrap::FireDocLoadingEvent(PRBool aIsFinished) where aIsFinished is PR_TRUE. 

We use that for Windows -- it catches the doc load complete signal every time.

Comment 10

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

Attachment: patch to support load-complete, reload, load-stopped

1.we'll support other document events later when there is a clear def and requirement of them
2.we'll add more specific paras when signal fire when there is a req later

Comment 11

[Aaron Leventhal]

Comment on attachment 237701 [details] [diff] [review]
patch to support load-complete, reload, load-stopped

There needs to be an EVENT_DOCUMENT_LOAD_START, right?
In fact the code in nsAccessibilityService sometimes doesn't even initialize the eventType variable, when STATE_START but not one of the reload types.

+  PRBool isFinished = 
+             (aEventType == nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE) ?
+             PR_TRUE : PR_FALSE;
+
+  if (!isFinished) {

Just change that to something like
if (aEventType == nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_STOP || 
    aEventType == nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE)

Comment 12

[Aaron Leventhal]

BTW, shouldn't they be document:load-complete, document:reload and document:load-stopped?

Comment 13

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

Attachment: patch v2

veirfied with at-poke. 

atk, at-spi and at-poke patch can be got from http://bugzilla.gnome.org/show_bug.cgi?id=356688

Comment 14

[Bill Haneman]

 What about these:
   * "document:content-changed" The contents of the Document container has
changed. [should be fired if container is re-used for new content, or if content is entirely replaced; Aaron should know if/when this happens...]
    * "document:attributes-changed" The global attributes (i.e. document-wide
attributes) of the Document have changed. [should be fired if the document's locale or other attributes exported via AtkDocument:getDocumentAttributes change]

(Perhaps document:attributes-changed should be the subject of another bug, or part of an RFE to support those Document-specific attributes?  Note that the attributes exported via AtkDocument:getDocumentAttributes are not the same as those from AtkObject:getAttributes, they are a subset which apply to "document" objects only.)

Comment 15

[Aaron Leventhal]

Comment on attachment 239128 [details] [diff] [review]
patch v2

Just glancing quickly here since I'm at an all day meeting.

We still need the EVENT_DOCUMENT_LOAD_START signal in order to not regress the windows behavior, corect? That would be an else { eventType = nsIAccessibleEvent::EVENT_DOCUMENT_START; } in the accservice changes.

Comment 16

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

(In reply to comment #15)
> (From update of attachment 239128 [details] [diff] [review] [edit])
> Just glancing quickly here since I'm at an all day meeting.
> 
> We still need the EVENT_DOCUMENT_LOAD_START signal in order to not regress the
> windows behavior, corect? That would be an else { eventType =
> nsIAccessibleEvent::EVENT_DOCUMENT_START; } in the accservice changes.
> 
what do you mean EVENT_DOCUMENT_LOAD_START here. the patch is no related to that

Comment 17

[Ginn Chen]

(In reply to comment #16)
> what do you mean EVENT_DOCUMENT_LOAD_START here. the patch is no related to
> that
> 

I think Aaron wants you to add it.

Comment 18

[Aaron Leventhal]

It is related, because currently for MSAA we fire a STATE_CHANGE event for the document both when loading starts and when it stops. With your last patch that will regression. You need to add a new internal signal to base the first state change event on. I suggest EVENT_DOCUMENT_LOAD_START.

Comment 19

[Aaron Leventhal]

In response to Bill:
  * "document:content-changed" 
There's no differentiation between major and minor DOM changes -- we would need to calculate that.

    * "document:attributes-changed" 
Currently none of the document attributes we have can actually change, other than DocURL, which can change if the user jumps to a named anchor (e.g. from www.mozilla.org to www.mozilla.org#maincontent)
Mimetime and doctype can't change during the life of a document.

Comment 20

[Bill Haneman]

Aaron - I understand about document attributes in mozilla.  But since we've said elsewhere that we want to handle locale changes via document-attribute-change events, I think we should still put the infrastructure in place for this (certainly ATK needs it).  Even though locale changes might not happen a lot, they could conceivably happen at the initial creation of a doc.

I guess by "content changed"/major change I would mean something that replaced either the entire DOM or something pretty close.  If that "never happens" in mozilla then perhaps you will never emit this event.

Comment 21

[Aaron Leventhal]

(In reply to comment #20)
> Aaron - I understand about document attributes in mozilla.  
The locale won't change for the entire document unless a new document is loaded. In that case the AT will see a load-complete change. That means an entirely new document exists, so nothing they knew about a previous document is relevant, including locale. Are you asking that an attribute-changed events gets fired for locale at the same time?

> or change I would mean something that replaced
> either the entire DOM or something pretty close.  If that "never happens" in
> mozilla then perhaps you will never emit this event.
We can calculate that, but we don't currently. I think it needs discussion in a separate bug. We can  do something for Firefox 3 if any the AT developers ask us for it.

Comment 22

[Aaron Leventhal]

Nian, for the attributes-change on DocURL, you can simply implement nsDocAccessibleWrap::FireAnchorJumpEvent for ATK. It's already implemented in nsDocAccessibleWrap in MSAA, so just declare it and implement it. That method will be called whenever the URL for the current doc changes.

Comment 23

[Aaron Leventhal]

Nian, when a new load starts, please fire a state change event on the document for STATE_BUSY.

Comment 24

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

Aaron, does my understanding below right? cu in irc
currentlly, 
for nsDocAccessible:
   1)not finished, do nothing
   2)finished, invalidate children
for atk/nsDocAccessible:
   1)not finished, fire EVENT_REORDER
   2)finished, do nothing
for msaa/nsDcoAccessible:
   1)not finished, fire EVENT_STATE_CHANGE
   2)finished, use timer call docLoadCallback

after implement:
for nsDocAccessible:
   1)not finished, fire EVENT_STATE_CHANGE
   2)finished, a)invalidate children b) user timer call docLoadCallback
for atk/nsDocAccessible:(fire document events)
   1)not finished, fire EVENT_REORDER?? or do nothing
   2)finished, do nothing
for msaa/nsDcoAccessible:
   1)not finished, do nothing
   2)finished, do nothing

we move msaa special part to nsDocAccessible.

questions are:
1.we still need a EVENT_REORDER for atk? if yes(which i think is right), so what a EVENT_STATE_CHANGE means for atk

2.for docLoadCallback, the commnets there in msaa seems for windows only, you're sure want to move it to nsDocAccessible?

3.your last comment. what do you mean "for STATE_BUSY"?

Comment 25

[Aaron Leventhal]

> after implement:
> for nsDocAccessible:
>   1)not finished, fire EVENT_STATE_CHANGE
>   2)finished, a)invalidate children b) user timer call docLoadCallback
Also call EVENT_STATE_CHANGE when finished, STATE_BUSY has been cleared

> for atk/nsDocAccessible:(fire document events)
>   1)not finished, fire EVENT_REORDER?? or do nothing
Don't fire EVENT_REORDER the same way we used to. See below.
Fire document:reload if it's from a reload
>    2)finished, do nothing
Fire document:load-complete or document:load-stopped depending on what happened

> for msaa/nsDcoAccessible:
>   1)not finished, do nothing
>   2)finished, do nothing
Correct

> we move msaa special part to nsDocAccessible.
Correct

> questions are:
> 1.we still need a EVENT_REORDER for atk? if yes(which i think is right), 
> so what a EVENT_STATE_CHANGE means for atk
When the old nsDocAccessible is destroyed and the new one is created, we should fire EVENT_REORDER not on the document, but on the parent of the document accessible, because EVENT_REORDER means, the children have changed. Since the doc accessible is a new pointer and a new object, that means the parent of it needs the event.

> 2.for docLoadCallback, the commnets there in msaa seems for windows only,
> you're sure want to move it to nsDocAccessible?
Just change the comments to make them cross platform.
Something like:
// Doc has finished loading, fire "load finished" events
// By using short timer we can wait until the is document is made  visible,
// which is done asynchronously on some systems. 
// This avoids confusing assistive technologies with a hidden document.
// Waiting also allows us to see if the document has focus,
// which is important because we only fire doc loaded events for focused
// documents.

> 3.your last comment. what do you mean "for STATE_BUSY"?
When you fire a state change event, you can specify what state has changed. The state that is changing is nsIAccessible::STATE_BUSY

Comment 26

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

Attachment: patchv3

not handle FireAnchorJumpEvent for atk side. do we need that? if yes, what's the event atk is interested in?

Comment 27

[Aaron Leventhal]

> not handle FireAnchorJumpEvent for atk side. do we need that? if yes, what's
> the event atk is interested in?

It should result in a
"document:attributes-changed" for DocURL

Comment 28

[Aaron Leventhal]

Comment on attachment 240300 [details] [diff] [review]
patchv3

This is no longer needed, in src/msaa or src/atk:
NS_IMETHODIMP nsDocAccessibleWrap::FireDocLoadingEvents(PRUint32 aEventType)
The only different line between those 2 is FireToolkitEvent() in the ATK version of that method, and you can put
that in nsDocAccessible::FireDocLoadingEvent now. We're going to fire those events on Windows now.

I don't think there is a document:load_start event in ATK. The EVENT_DOCUMENT_LOAD_START exists just for internal reasons, so that the state change event gets fired.
So I'm not sure why that's getting fired -- it looks like our invention.

Finally, please add support for the anchor jump atributes-changed event for the DocURL attribute, or at least file a separate bug for it.

+  PRBool isFinished =   
+             (aEventType == nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE ||
+              aEventType == nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_STOPPED) ?
+             PR_TRUE : PR_FALSE;
Nit, this could be just:
  PRBool isFinished = aEventType == nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE ||
                      aEventType == nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_STOPPED;

Comment 29

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

Attachment: patch v4

1.addressing aaron's comment(a)not fire doc_load_start, b)merge FireDocLoadEvents into nsDocAccessible, c)nit part

2.support doc-url change for atk(a)move common part into nsDocAccessible, b)fixed bug when jump between anchors, c)fire attributes-changed for atk

Comment 30

[Aaron Leventhal]

Comment on attachment 240403 [details] [diff] [review]
patch v4

Nian, please fire the EVENT_DOCUMENT_ATTRIBUTES_CHANGED in the cross platform code,  so you don't need mAnchorJumped.

Explanation: that event doesn't exist in Windows yet, but we are going to start defining custom event constants for MSAA and fire it anyway starting in Firefox 3.

My apologies for not saying this earlier.

Looking for 2nd review from Ginn since this is a large, important patch and we need to make sure it is right.

Comment 31

[Ginn Chen]

Comment on attachment 240403 [details] [diff] [review]
patch v4

+  PRUint32 eventType = -1;
This is bad.

It will cause compiler warning, and 0xffff is used for nsIAccessibleEvent::EVENT_INTERNAL_LOAD.

I suggest to use 0 instead.
Also I'd like to move 
+  if (aEventType == -1)
+    return NS_OK;  // no actural event need to be fired
to
nsAccessibilityService.cpp

Comment 32

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

Attachment: patchv5

adress aaron and ginn's comment.

mAnchorJumped is still needed for msaa

also move AtkStateChange to StateChange

Comment 33

[Håkan Waara]

The nsPIAccessibleDocument interface was changed, and should probably therefore also be rev'd.

Comment 34

[Ginn Chen]

(In reply to comment #33)
> The nsPIAccessibleDocument interface was changed, and should probably therefore
> also be rev'd.
> 

Fixed, thanks!