494802 - Caret-moved events missing from Location bar

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Joanmarie Diggs]

Steps to reproduce:

1. Set your preferences so that when Firefox starts, it uses the windows and tabs from last time.

2. Navigate to mozilla.org making sure focus is not in an entry.

3. Quit Firefox.

4. Launch Accerciser and use the event monitor to monitor object:text-caret-moved events for all applications.

5. Launch Firefox.

6. Press Control L to move focus to the Location bar.

7. Type some text and arrow around.

8. Navigate to google.com, leaving focus in the page's search entry.

9. Quit and relaunch Firefox.

10. Type some text and arrow around.

Expected results: object:text-caret-moved events would be present after steps 7 and 10.

Actual results: object:text-caret-moved events are only present after steps 10. For some reason, if Firefox is launched when focus is in an entry, the Location bar will subsequently generate the appropriate events; otherwise it will not.

Impact: Without the caret-moved events, the Location bar is inaccessible for users who are blind.

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Thanks.

(In reply to comment #0)
> For some reason, if Firefox is launched when focus is in an entry, the Location
> bar will subsequently generate the appropriate events

Sounds intriguing, but to be clear, what does "launched when focus is in an entry" mean exactly?

Comment 2

[Joanmarie Diggs]

(In reply to comment #1)
> Thanks.
> 
> (In reply to comment #0)
> > For some reason, if Firefox is launched when focus is in an entry, the Location
> > bar will subsequently generate the appropriate events
> 
> Sounds intriguing, but to be clear, what does "launched when focus is in an
> entry" mean exactly?

Some sites (like Google) cause the caret to be automatically placed in an editable text field (ROLE_ENTRY) upon displaying the page. In addition, if you set your Firefox preferences so that Firefox starts out with a blank page, focus is in the Location autocomplete (whose focused child is of ROLE_ENTRY) so that you can start typing a URL.

Put another way, this bug can be summarized as "If you see a blinking cursor when you first launch Firefox, things should be okay." ;-) ;-) ;-)

Does this make sense (not the bug, which is odd; but what I'm trying to report)?

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Yep just wanted to be clear thanks! It is sometimes these sort of strange bits that can help diagnose the cause.

Comment 4

[Marco Zehe (:MarcoZ)]

Was this introduced recently? It sounds like a recent regression, although I currently can't imagine which of our bug fixes could have caused caretMoved events to break.

Comment 5

[Ginn Chen]

I can reproduce this bug.
The summarize of "If you see a blinking cursor when you first launch Firefox, things should be okay." is not quite accurate.
If I type "abcd" in google search entry, and then kill the firefox-bin process.
Restart Firefox, it will restore the page and "abcd" in the entry, I can see caret-moved(4,0, None) in accerciser, but I can't see caret-moved event for location bar. ;-) ;-) ;-)

Comment 6

[Ginn Chen]

I think the root cause if it thought nsRootAccessible is still BUSY in nsCaretAccessible::NotifySelectionChanged.
It can be verified in accerciser.

Is it a regression?

I have never reached nsRootAccessible::FireDocLoadEvents().
So the state is decided by when we construct the nsRootAccessible, kind of random.

Perhaps we miss used nsIWebProgressListener.

Comment 7

[Ginn Chen]

I guess we don't really care the document load events or busy state of nsRootAccessible.
We can just make it always not busy.

Comment 8

[Ginn Chen]

BTW: mIsContentLoaded or Busy flag should not prevent caret-moved event of location bar anyway.

Comment 9

[Ginn Chen]

Attachment: 494802

Comment 10

[Marco Zehe (:MarcoZ)]

I tested this patch on Windows, and it doesn't appear to break any screen readers. Seems they're not dependent at all on the root accessibles firing the busy etc. events.

Comment 11

[alexander :surkov (:asurkov)]

Historical note.

Ginn's patch is kind of backing out of bug 366520 to bug 310350. Bug 310350 marked all chrome documents as loaded when nsDocAccessible is created. Bug 366520 tried to fix excess cache invalidations of chrome documents when we got load events from webprogress listener (therefore nsRootAccessible::FireDocLoadEvents was introduced). We started to mark document as loaded when we get load event for chrome document.

Ginn noticed the problem might be in nsIWebProgressListener for chrome documents so we don't know chrome document is loaded when we try to fire caret change events. If chrome document is always loaded when nsDocAccessible is created then we should set mIsContentLoaded to true always for chrome documents (not for root chrome documents only like in Ginn's patch). Also we should keep in mind bug 404380 where we got craches on caret change events because document was not loaded. If we could ensure bug 404380 takes a place for content documents only then Ginn's approach looks correct in general.

Comment 12

[Ginn Chen]

For the nsIWebProgressListener, we listen to nsIWebProgress::NOTIFY_STATE_DOCUMENT | nsIWebProgress::NOTIFY_LOCATION.
But for the chrome document, these things won't change, and nsRootAccessible::FireDocLoadEvent will not be called.
We have the busy flag for root window because there're network transmissions.
If we really want to listen the events for root window, we should try NOTIFY_STATE_NETWORK and/or NOTIFY_STATE_WINDOW, NOTIFY_STATUS.
I don' think it's necessary since we don't emit document load event for root window anyway.

Comment 13

[alexander :surkov (:asurkov)]

Kind of general question. Why do we need to fire load events? How does AT use them? It makes me bother if we won't fire load events for chrome documents.

Comment 14

[alexander :surkov (:asurkov)]

(In reply to comment #13)
> Kind of general question. Why do we need to fire load events? How does AT use
> them? It makes me bother if we won't fire load events for chrome documents.

Joanmarie, James?

Comment 15

[Joanmarie Diggs]

(In reply to comment #13)
> Kind of general question. Why do we need to fire load events? How does AT use
> them? It makes me bother if we won't fire load events for chrome documents.

So that ATs know that new content has been loaded on screen which they might then want to present.

Comment 16

[alexander :surkov (:asurkov)]

(In reply to comment #15)
> (In reply to comment #13)
> > Kind of general question. Why do we need to fire load events? How does AT use
> > them? It makes me bother if we won't fire load events for chrome documents.
> 
> So that ATs know that new content has been loaded on screen which they might
> then want to present.

(In reply to comment #12)
> I don' think it's necessary since we don't emit document load event for root
> window anyway.

Ginn, so how does AT know the chrome document was loaded?

Comment 17

[James Teh [:Jamie]]

(In reply to comment #13)
> Kind of general question. Why do we need to fire load events? How does AT use
> them? It makes me bother if we won't fire load events for chrome documents.
Sorry for the slow response. We use load events for content documents if the document gained focus before it was finished loading. In this case, we need the load event to know we can start processing the document. If the document is already loaded at the time we gain focus, we don't use the load event at all. I'm not quite sure what is and isn't a chrome document in Firefox, but I"m guessing we don't need load events for chrome docs at all.

Comment 18

[alexander :surkov (:asurkov)]

James, do you check STATE_BUSY to know whether the document was loaded?

Comment 19

[James Teh [:Jamie]]

For content documents, yes... but I'm assuming this isn't a content document.

Comment 20

[alexander :surkov (:asurkov)]

(In reply to comment #19)
> For content documents, yes... but I'm assuming this isn't a content document.

How do you know this content document and this chrome document?

Comment 21

[alexander :surkov (:asurkov)]

(In reply to comment #12)
> For the nsIWebProgressListener, we listen to
> nsIWebProgress::NOTIFY_STATE_DOCUMENT | nsIWebProgress::NOTIFY_LOCATION.
> But for the chrome document, these things won't change, and
> nsRootAccessible::FireDocLoadEvent will not be called.

Ginn, I can see FireDocLoadEvent are called for chrome documents twice, the first time is EVENT_DOCUMENT_LOAD_START, the second time is EVENT_DOCUMENT_LOAD_STOPPED.

Comment 22

[alexander :surkov (:asurkov)]

(In reply to comment #7)
> I guess we don't really care the document load events or busy state of
> nsRootAccessible.
> We can just make it always not busy.

I think nsRootAccessible can be easy created for non chrome document, for example, openDialog("http://")

Comment 23

[alexander :surkov (:asurkov)]

Ginn, probably the right way is to fire text change events for entry always because it means text field is ready for work. We have bug 382021 for this.

Comment 24

[alexander :surkov (:asurkov)]

fixed by bug 566103, confirmed by Ginn in bug 566103 comment #37.