Closed Bug 420845 Opened 17 years ago Closed 15 years ago

Fire event_reorder on any embedded frames/iframes whos document has just loaded.

Categories

(Core :: Disability Access APIs, defect, P1)

x86
Windows Vista
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: mick, Assigned: surkov)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008030305 Minefield/3.0b4pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008030305 Minefield/3.0b4pre So that ATs know that a subtree has changed, nodes fire an event_reorder to say that its children have some how changed. For nodes that only contain text and the text changes, text_inserted/text_removed is fired. This means that ATs can simply watch for events that tell it to re-render part of the tree in their own cached implementations. However, iframes and frames do not fire an event to indicate that its children has changed (e.g. when a document is removed or added). If an event_reorder was fired on any iframe/frame in a document, when its own document was loaded, this would make it much easier for ATs such as NVDA to track changes such as this. It could be possible to just track show and hide events, and then go up the parents or use node_child_of relation, however at this time in NVDA its much easier to just re-render subtrees than individual nodes. Reproducible: Always Steps to Reproduce: 1. 2. 3.
This is an important efficiency fix which we should implement if possible.
Blocks: fox3access
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9?
Priority: -- → P1
Version: unspecified → Trunk
Ok, so it's an efficiency fix, but should it block the release?
Poke. Answer for question in comment #2?
Michael, we don't fire EVENT_DOCUMENT_LOAD_COMPLETE and etc event for documents contained by frames/iframes or arent' they helpful?
Damon, i think screen readers like NVDA can get around this. Marco requested it though, so maybe he can explain why. Mick, can you use the new NODE_CHILD_OF relation we put on the frame/iframe objects?
(In reply to comment #5) > Mick, can you use the new NODE_CHILD_OF relation we put on the frame/iframe > objects? Yes, we can use it, and probably will implement that after CSUN. However, I still think that allowing iframes to fire event_reorder is just making the over all pattern of events more complete. As far as I understand, Gecko fires textInserted / textRemoved for any Accessible object that contains text, as it changes. But, for any accessible object that does not have text, and its children change in some way, an event_reorder is fired. However, this is not true for frames/iframes. If these were fired for frames/iframes, NVDA's dynamic content support would become pretty much complete, in that if a new document is loaded in a frame/iframe due to an action on the page, NVDA would re-render the frame and therefore display the new sub document. This does not currently happen. We could use node_child_of, but the code would be a specific exception to the event_reorder rule. I guess this is not a really really important issue as we can use node_child_of, but I still believe that adding this for frames/iframes, completes the use of reorder on accessible objects in Gecko.
(In reply to comment #2) > Ok, so it's an efficiency fix, but should it block the release? See Mick's explanation of the advantage it would have in comment #6. I would say it is at least a very very trong WANTED, because we should try and help the NVDA developers to get the best support possible, and to me it sounds like using relation_child_of is a hoop they'd have to go through because of our incomplete implementation. Aaron, Surkov, how difficult would it be to implement this?
Given this can be worked around taking off the blocker list. We'd take a fix now or in a 0.x release.
Flags: blocking1.9? → blocking1.9-
Mick & Jamie, we never got around to fixing this, and I haven't heard you complain. That makes me think we don't need to fix it. It might make things more consistent but we have so many other bugs that are actually affecting users. Let me know if you feel strongly one way or another.
NVDA does not depend on this bug specifically. Although as I have previously stated, currently there must be an exception made for documents in frames and iframes. This bug certainly isn't overly important right now. But I wouldn't like to see it get closed because of that. All objects fire event_reorder if a child is added or removed, except for iframes and frames. Either: *Fire an event_reorder when a document in the frame loads, or *Fire an event_reorder when the new document object is created, and then fire event_document_load_complete on the document when its fully loaded.
Is this still an issue after bug 472662 landed?
(In reply to comment #11) > Is this still an issue after bug 472662 landed? Yes. We still get no reorder event on frames/iframes in: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090307 Minefield/3.2a1pre.
For the record, document load complete isn't fired on the document either. Thankfully, we do get a state change, so this is what we are using in our reimplementation of NVDA's virtual buffer code.
Mass un-assigning bugs assigned to Aaron.
Assignee: aaronleventhal → nobody
Attached patch patchSplinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #388673 - Flags: review?(marco.zehe)
Attachment #388673 - Flags: review?(bolterbugz)
Comment on attachment 388673 [details] [diff] [review] patch One nit: >+ <title>Accessible documentation events testing</title> Should be "document events", not "documentation", correct? Related question: Is there a way for us to test the loading/state-busy, loaded/busy flag cleared, refreshing/state-busy etc. states we normally fire for a document load so we have test coverage for these? Or should I file a new bug for those?
Comment on attachment 388673 [details] [diff] [review] patch r=me with the nit in comment #16 addressed. I also tested this patch and it appears to have no negative effects on ATs.
Attachment #388673 - Flags: review?(marco.zehe) → review+
(In reply to comment #16) > Related question: Is there a way for us to test the loading/state-busy, > loaded/busy flag cleared, refreshing/state-busy etc. states we normally fire > for a document load so we have test coverage for these? Or should I file a new > bug for those? we should listen events I think and test states. iirc we have bugs here. It's worth to deal in another bug.
(In reply to comment #16) > (From update of attachment 388673 [details] [diff] [review]) > One nit: > >+ <title>Accessible documentation events testing</title> > > > Should be "document events", not "documentation", correct? right, I guess I tired a bit. I replaced on "Accessible events testing for document" locally.
James, Mick, could you try the try-server build to ensure this suites your needs?
(In reply to comment #21) > James, Mick, could you try the try-server build to ensure this suites your > needs? Works very well. Thanks!
Attachment #388673 - Flags: review?(bolterbugz) → review+
Comment on attachment 388673 [details] [diff] [review] patch r=me; thanks. Feels like we should document this somewhere. Do we have an AT guide? >+++ b/accessible/src/base/nsDocAccessible.cpp >@@ -612,17 +612,27 @@ nsDocAccessible::Init() >+ FireDelayedAccessibleEvent(reorderEvent); >+ return NS_OK; > } So we'll fire a reorder even for the main document? I guess that's ok.
(In reply to comment #23) > (From update of attachment 388673 [details] [diff] [review]) > r=me; thanks. > > Feels like we should document this somewhere. Do we have an AT guide? mark it as dev-doc-needed. > >+ FireDelayedAccessibleEvent(reorderEvent); > >+ return NS_OK; > > } > > So we'll fire a reorder even for the main document? I guess that's ok. probably, it would be more logical to put it in parent's queue but this requires one more step to get parent's document :) The current approach is also ok.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED
Blocks: 506206
Now listed in the notes on Firefox 3.6 for developers: https://developer.mozilla.org/en/Firefox_3.6_for_developers
The docs now have this phrase: "The reorder event is now sent to embedded frames and iframes when their document is loaded. See bug 420845." Alexander, David, could you clarify the documentation or at least comment here, please? To a naive reader of that page it reads as "an alias for the DOM "load" event was added", which is probably far from being true.
(In reply to comment #27) > The docs now have this phrase: "The reorder event is now sent to embedded > frames and iframes when their document is loaded. See bug 420845." > > Alexander, David, could you clarify the documentation or at least comment here, > please? To a naive reader of that page it reads as "an alias for the DOM "load" > event was added", which is probably far from being true. You're right reorder event is not alias of the load event. Load event is fired on document accessible when document was loaded. Reorder event is fired on document container accessible when document accessible was attached to it. So that reorder event is used to notify the AT the subtree was changed. Load event is used to notify the AT the content of the document is ready (we don't fire any mutation events like reorder, show, hide for the document content prior this point). The current implementation makes fire reorder event before load event. I think technically AT would be able to use load event instead of reorder event in this case but they find the reorder event more comfortable because in this case they don't need any special processing of document container accessible. Iirc that was an original point of the bug.
OK, is it a DOM event? Content-accessible or chrome-only? Fired only when accessibility is enabled? In other words, what kind of developers may be interested in this? I have a strong suspicion this should have been added to accessibility APIs documentation only and isn't interesting to the general web developer/extension developer population. Am I wrong?
You're right. It's a11y event. Usually developer/extension don't need this until they create accessibility tool. In gecko all a11y events can be watched by nsIObserverService service. As well internal gecko a11y events result system events are fired on various platforms, for example, Win API NotifyWinEvent function is used to fire a MSAA event.
Thanks a lot for the clarification! I clarified the text on https://developer.mozilla.org/en/Firefox_3.6_for_developers (at least making it clear it's about accessibility). EVENT_REORDER is also mentioned on a few other pages, which may need updating, I'm not sure: https://developer.mozilla.org/Special:Search?search=EVENT_REORDER&type=fulltext&go=Search
(In reply to comment #31) > Thanks a lot for the clarification! I clarified the text on > https://developer.mozilla.org/en/Firefox_3.6_for_developers (at least making it > clear it's about accessibility). I guess it would make sense to have own section for accessibility changes. > EVENT_REORDER is also mentioned on a few other pages, which may need updating, > I'm not sure: > https://developer.mozilla.org/Special:Search?search=EVENT_REORDER&type=fulltext&go=Search Our accessibility document needs to be updated and possibly rewritten from the start.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: