547506 - Many duplicate object:text-changed:insert events emitted for the Location bar when Orca is not run in async mode

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect)

Description

[Joanmarie Diggs]

Step to reproduce:

1. Attempt to run the Orca regression tests for Firefox using Firefox 3.6 from 28th July or later.

Expected results: The tests would complete successfully.

Actual results: Each and every test in which a URI is inserted grinds to a halt when the second character is inserted into the Location bar.

Looking at the debug output, we are getting an excessive number of identical object:text-changed:insert events for the second character of the URI being entered for that test. By excessive, I mean I expect to get one; I get in the neighborhood of 60. At first I thought perhaps the number coincided with the number of characters in the URL. It does not. The number of duplicate events emitted exceeds that -- in some cases by around 15 to 20.

Looking at the changes from that period, my *guess* is that this might be a side effect of the fix for bug 504384. I've not actually built Firefox to test this theory, however.

Regardless.... As things currently stand, we can no longer use Orca's regression tests for Firefox. :-(

Other observations:

I've tried manually running Orca with orca.settings.asyncMode set to False and was able to reproduce the problem a couple of times. Most of the time, I cannot reproduce the problem manually, however.

Seems to me that our choices are:

* Y'all stop the flood of accessible events (please, please, please) :-)

* We change our regression test harness and/or try some other way to detect and ignore the deluge within Orca (CCing Will for his thoughts)

* We make special builds of Firefox for the purpose of testing with Orca. (assuming that bug 504384 is indeed the winner, the change seems isolated: http://hg.mozilla.org/mozilla-central/rev/a846e4cb810c)

Any chance you guys can stop the flood? (please, please, please) :-) 'Cause, at least from where I'm sitting, the latter two options seem far less ideal.

Thanks!

Comment 1

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

(In reply to comment #0)

> Looking at the debug output, we are getting an excessive number of identical
> object:text-changed:insert events for the second character of the URI being
> entered for that test. By excessive, I mean I expect to get one; I get in the
> neighborhood of 60.

Well that's just silly, we should stop doing that! :)
 
> * Y'all stop the flood of accessible events (please, please, please) :-)

Yep and when we do, we need to fix this at the source. We've discovered that our event processing is a hot path and trying to smartly filter can grind firefox to a halt for highly mutating DOM's. From a system perspective I'd prefer the AT decide whether to queue and filter internally. That said, I still think FF has a bug here.

> I've not actually built Firefox to test this theory, however.

Build to test your hunch should appear here soonish:
https://build.mozilla.org/tryserver-builds/[e-mail]-try-740fa04500d7/

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> (In reply to comment #0)

>  From a system perspective I'd
> prefer the AT decide whether to queue and filter internally. That said, I still
> think FF has a bug here.
> 

I would say somebody one should filter events. We shouldn't do this both. Since Firefox do this then we should keep this way I think.

Comment 3

[Joanmarie Diggs]

(In reply to comment #1)
> (In reply to comment #0)
> 
> > Looking at the debug output, we are getting an excessive number of identical
> > object:text-changed:insert events for the second character of the URI being
> > entered for that test. By excessive, I mean I expect to get one; I get in the
> > neighborhood of 60.
> 
> Well that's just silly, we should stop doing that! :)

Awesome. Beer's on me at CSUN! ;-)

> > I've not actually built Firefox to test this theory, however.
> 
> Build to test your hunch should appear here soonish:
> https://build.mozilla.org/tryserver-builds/[e-mail]-try-740fa04500d7/

Thanks so much for doing that! Unfortunately, using your build, I'm still seeing the problem. :-(

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #1)

> Build to test your hunch should appear here soonish:
> https://build.mozilla.org/tryserver-builds/[e-mail]-try-740fa04500d7/

David, would be nice if you give a hint about the changes. It might be helpful. If you're trying to play with event coalescence algorithm then we don't coalesce text events iirc.

Comment 5

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

(In reply to comment #4)
> (In reply to comment #1)
> 
> > Build to test your hunch should appear here soonish:
> > https://build.mozilla.org/tryserver-builds/[e-mail]-try-740fa04500d7/
> 
> David, would be nice if you give a hint about the changes.

I was just trying out Joanie's hunch and added this._notifyResults(true); back to toolkit/components/places/src/nsPlacesAutoComplete.js. (Yeah it was a long shot)

We need to debug this and find out why we get dup text events.

Comment 6

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

I profiled about 5 seconds of typing in the awesome bar. A lot of time is spent in nsXULElement:RemoveChild (about 4.7%) and nsXULElement::AppendChild (about 0.7%). We create a lot of nsAccTextChangeEvents (in calls to nsDocAccessible::CreateTextEventForNode).

I did some trace output and I see roughly 30 instances of creating these events when typing a second letter (inserts + removals). We also fire 

Aside: we do fire text change events synchronously; no coalescing.

Each time we add/remove a letter in the awesomebar the list of best matches changes. This probably causes a lot of events. I wonder if there is a way we can identify the start and end points of a single batch of awesomebar node changes.

Joanie, just to be clear, you are getting about 60 identical object:text-changed:insert for the second typed letter?

Comment 7

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

Note as Joanie says these are not the object:text-changed:insert:system events (which would be ignored because of the "system" suffix) -- those text inserts are caused by the awesomebar (suggestions).

Comment 8

[Joanmarie Diggs]

Any updates?

I'm hesitant to make changes in Orca which might break Orca's access to Gecko-based apps. It would be nice to have this bug fixed so that I can resume running Orca's regression tests with current versions of FF.

Thanks!

Comment 9

[Joanmarie Diggs]

I'm not seeing this bug in:

   Mozilla/5.0 (X11; Linux i686; en-US; rv:2.0b2pre)
   Gecko/20100719 Minefield/4.0b2pre

Did the bug fairies fix it whilst you were sleeping? :-)

Assuming my good fortune is permanent and not a temporary accident, this bug can be closed out. I'll just use 4.0 for testing purposes.

Comment 10

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

I love bug fairies!

Thanks for following up on this one Joanie -- closing WFM.