718237 - [SeaMonkey] "accessible/events/test_focus_autocomplete.xul | Test timed out." (which also causes lots of "gA11yEventListeners is undefined" on following tests)

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect)

Description

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326546128.1326550668.4721.gz
Linux comm-central-trunk debug test mochitest-other on 2012/01/14 05:02:08
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1326518958.1326523475.2710.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/01/13 21:29:18
{
2656 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_focus_autocomplete.xul | Test timed out.
}

SM 2.8 (Aurora) and 2.7 (Beta) are affected too.

Comment 1

[Serge Gautherie (:sgautherie)]

Ftr, this is also causing LOTS of
{
[...] ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/[...] | an unexpected uncaught JS exception reported through window.onerror - gA11yEventListeners is undefined at chrome://mochitests/content/a11y/accessible/events.js:1438
}
on "all" following tests.

Comment 2

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows NT 5.0; rv:12.0a1) Gecko/20120104 Firefox/12.0a1 SeaMonkey/2.9a1]

Opt build, local run, this test only:

If I let it timeout:
{
14 INFO TEST-PASS | chrome://mochitests/content/a11y/accessible/events/test_focu
s_autocomplete.xul | test with ID = ' 'autocomplete'  'down ' key' failed. There
 is unexpected focus event.

15 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/even
ts/test_focus_autocomplete.xul | Test timed out.
}

***

If, while it is timing out, I focus another window then focus the test window back:
{
15 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/even
ts/test_focus_autocomplete.xul | an unexpected uncaught JS exception reported th
rough window.onerror - controller.input is null at chrome://mochitests/content/a
11y/accessible/events/test_focus_autocomplete.xul:258
}

Code would be:
{
253       // XUL autocomplete
254       var popupNode = autocompleteNode.popup;
255       if (!popupNode) {
256         // HTML form autocomplete
257         var controller = Components.classes["@mozilla.org/autocomplete/controller;1"].
258           getService(Components.interfaces.nsIAutoCompleteController);
259         popupNode = controller.input.popup.QueryInterface(nsIDOMNode);
260       }
}

But I am not sure whether this 'controller.input is null' error is an artefact or the actual cause :-|

Fwiw, notice "@mozilla.org/autocomplete/controller;1" is defined in /toolkit/components/build/nsToolkitCompsCID.h (only).
http://mxr.mozilla.org/mozilla-central/search?string=%40mozilla.org%2Fautocomplete%2Fcontroller%3B1&case=on

***

I would guess this bug may be another XPFE vs Toolkit autocomplete issue.
http://mxr.mozilla.org/mozilla-central/find?text=&string=autocomplete.xml
But I am not sure what to check (or how to fix) exactly.

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) accessible/tests/mochitest/*: Support "SeaMonkey searchbar" too, Remove 1 useless "browser.css" use [Checked in: Comment 5]

events/test_focus_autocomplete.xul:
Removing browser.css would trigger "this" bug on FF :->
I assume the 'searchbar' element needs it...
See https://tbpl.mozilla.org/?tree=Try&rev=a9c7511bd8e3
Adding navigator.css for SM doesn't fix this bug. (Not enough?)
Anyway, it is more correct to have it for SM too, as there is a 'searchbar' element.

focus/test_takeFocus.xul:
Afaict, browser.css isn't needed at all, is it?
Succeeded as https://tbpl.mozilla.org/?tree=Try&rev=3c6f3439600f
Let's remove it.

states/test_expandable.xul:
Test pass even without browser.css.
See https://tbpl.mozilla.org/?tree=Try&rev=3c6f3439600f
Maybe because some "gQueue.push(new *);" are commented out?
Anyway, it is more correct to have it, as there is a 'searchbar' element.
Same analysis for SM.

Comment 4

[alexander :surkov (:asurkov)]

Comment on attachment 588756 [details] [diff] [review]
(Av1) accessible/tests/mochitest/*: Support "SeaMonkey searchbar" too, Remove 1 useless "browser.css" use
[Checked in: Comment 5]

Review of attachment 588756 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, comment #3 is reasonable

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 588756 [details] [diff] [review]
(Av1) accessible/tests/mochitest/*: Support "SeaMonkey searchbar" too, Remove 1 useless "browser.css" use
[Checked in: Comment 5]

https://hg.mozilla.org/mozilla-central/rev/eb0fb073ac35

Comment 6

[Serge Gautherie (:sgautherie)]

Some clues after testing a little on SeaMonkey and comparing with Firefox logs:

SeaMonkey reports
{
search "test-a11y-search" not found - skipping.
search "test-a11y-search" not found - skipping.
}
which is
http://mxr.mozilla.org/comm-central/source/mozilla/xpfe/components/autocomplete/resources/content/autocomplete.xml#117
{
118       <method name="initAutoCompleteSearch">
...
134               dump("search \"" + name + "\" not found - skipping.\n");
}
Iiuc, the test's "initAutoComplete()" fails to register the search results.

Then the test fails when actually executing
|queueAutoCompleteTests("autocomplete");|
at step
"// select second option ('hi' option), focus on it"
which I can confirm as manually pressing Down to open the search result does nothing.

I don't know whether this should work as is after bug 558589 is fixed,
or if some SeaMonkey specific adaptation is needed.

PS: 'states/test_expandable.xul' is probably unaffected because its related code is commented out.

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) Skip this test on Seamonkey ftb.

Let's unbreak the whole(!) a11y suite, until we have a fix.

Comment 8

[[e-mail]]

There are two issues here:
1. XPFE autocomplete initialises its list of search sessions in its constructor; toolkit autocomplete is lazy and doesn't bother until it actually needs something to autocomplete. I think you can work around that by registering the autocomplete component at the top-level rather than after the load event.
2. XPFE autocomplete does nothing if there is no text in the widget, unlike toolkit autocomplete, which simply requests a search for the empty string. (Search sessions can react in whichever way they feel appropriate.)

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 598642 [details] [diff] [review]
(Bv1) Skip this test on Seamonkey ftb.

Review of attachment 598642 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/tests/mochitest/events/test_focus_autocomplete.xul
@@ +381,5 @@
>  
>      var gInitQueue = null;
>      function initTests()
>      {
> +      if (navigator.userAgent.match(/ SeaMonkey\//)) {

would you create a constant for that in common.js like we have constants for os? For example,
const SEAMONKEY = navigator.userAgent.match(/ SeaMonkey\//);
it appears that's not unique case where mochitests differs for Firefox and Seamonkey.

Comment 10

[[e-mail]]

3. XPFE autocomplete checks the results returned by the autocompletesearch provider and bails if the search string doesn't match.

Note: it's not yet clear whether you need to skip the whole test or just the xul autocomplete parts.

Comment 11

[[e-mail]]

And even if I tweak the autocomplete to fix 1, 2, and 3 above, it still gets stuck; this time apparently its synthetic Enter fails to close the popup.

Comment 12

[Serge Gautherie (:sgautherie)]

Attachment: (Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb [Checked in: Comments 13 and 17]

Bv1, with comment 9 suggestion(s),
and fixing a nit.


@ Neil:
Fwiw, I just checked enabling each of the 5 parts of doTests() individually: they all hang at some point.
I'm fine with waiting for bug 558589 first.

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 598719 [details] [diff] [review]
(Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb
[Checked in: Comments 13 and 17]

https://hg.mozilla.org/mozilla-central/rev/561771f01881


[Approval Request Comment]
Regression caused by (bug #): (old)
User impact if declined: None, but a11y suite completely broken on SeaMonkey.
Testing completed (on m-c, etc.): This comment.
Risk to taking this patch (and alternatives if risky): None, test only.
String changes made by this patch: None.

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to Serge Gautherie (:sgautherie) from comment #13)
> https://hg.mozilla.org/mozilla-central/rev/561771f01881

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329726992.1329731718.29744.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/02/20 00:36:32

V.Fixed wrt this workaround only.

Comment 15

[Alex Keybl [:akeybl]]

Comment on attachment 598719 [details] [diff] [review]
(Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb
[Checked in: Comments 13 and 17]

[Triage Comment]
Test only change - approved for Aurora 12 and Beta 11.

Comment 16

[Serge Gautherie (:sgautherie)]

Jens, can you do this c-n too? (I/We miss it because this bug is not assigned to me...)

Comment 17

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 598719 [details] [diff] [review]
(Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb
[Checked in: Comments 13 and 17]

http://hg.mozilla.org/releases/mozilla-aurora/rev/65f8a8f3283f
http://hg.mozilla.org/releases/mozilla-beta/rev/4fd7a33661c8

Comment 18

[Serge Gautherie (:sgautherie)]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #17)

> http://hg.mozilla.org/releases/mozilla-aurora/rev/65f8a8f3283f

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1330071749.1330076464.18395.gz
WINNT 5.2 comm-aurora debug test mochitest-other on 2012/02/24 00:22:29

> http://hg.mozilla.org/releases/mozilla-beta/rev/4fd7a33661c8

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1330085913.1330089588.19091.gz
WINNT 5.2 comm-beta debug test mochitest-other on 2012/02/24 04:18:33

firefox11 and firefox12: verified wrt this disabling patch only.

Comment 19

[Justin Wood (:Callek)]

Set status to reflect Bv2 landing on branches.

Comment 20

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) Call initAutoComplete() early to support XPFE AutoComplete [Checked in: Comment 27]

Fixes comment 8 point 1 (only):
no more "search "test-a11y-search" not found - skipping." errors.

Comment 21

[alexander :surkov (:asurkov)]

Comment on attachment 603126 [details] [diff] [review]
(Cv1) Call initAutoComplete() early to support XPFE AutoComplete
[Checked in: Comment 27]

Review of attachment 603126 [details] [diff] [review]:
-----------------------------------------------------------------

This looks like a workaround of the problem. So I wonder shouldn't the autocomplete widget be fixed instead?

Comment 22

[Serge Gautherie (:sgautherie)]

(In reply to alexander :surkov from comment #21)
> This looks like a workaround of the problem. So I wonder shouldn't the
> autocomplete widget be fixed instead?

(I concur, but)
Neil always replied that this can't be changed :-|

Comment 23

[alexander :surkov (:asurkov)]

Comment on attachment 603126 [details] [diff] [review]
(Cv1) Call initAutoComplete() early to support XPFE AutoComplete
[Checked in: Comment 27]

(In reply to Serge Gautherie (:sgautherie) from comment #22)
> (In reply to alexander :surkov from comment #21)
> > This looks like a workaround of the problem. So I wonder shouldn't the
> > autocomplete widget be fixed instead?
> 
> (I concur, but)
> Neil always replied that this can't be changed :-|

Ok, Neil knows what he talks about :)

We can fix mochitests but I'd bet one day this problem will pop up in other place.

Comment 24

[[e-mail]]

It's mostly because we still have to support LDAP, which relies on the widget being constructed eagerly so that it can add its component afterwards.

Comment 25

[alexander :surkov (:asurkov)]

(In reply to [e-mail] from comment #24)
> It's mostly because we still have to support LDAP, which relies on the
> widget being constructed eagerly so that it can add its component afterwards.

then some observing mechanism can be introduced I think so the widget gets updated whenever something new appears.

Comment 26

[[e-mail]]

What I meant is that only once LDAP has been switched to be a toolkit search session will we be able to remove all of the hacks we have that support the current version thus allowing us to rewrite the widget to initialise lazily.

Comment 27

[Serge Gautherie (:sgautherie)]

Comment on attachment 603126 [details] [diff] [review]
(Cv1) Call initAutoComplete() early to support XPFE AutoComplete
[Checked in: Comment 27]

https://hg.mozilla.org/mozilla-central/rev/bcbdc2a0efa1

Comment 28

[Serge Gautherie (:sgautherie)]

(In reply to [e-mail] from comment #8)
> 2. XPFE autocomplete does nothing if there is no text in the widget, unlike
> toolkit autocomplete, which simply requests a search for the empty string.
> (Search sessions can react in whichever way they feel appropriate.)

(In reply to [e-mail] from comment #10)
> 3. XPFE autocomplete checks the results returned by the autocompletesearch
> provider and bails if the search string doesn't match.

Neil, should we file a bug to change XPFE autocomplete behavior? (Or Toolkit one?)
Or should we update test(s) that depend on Toolkit behavior?

I don't know how non-Mozilla browsers behave...

Comment 29

[[e-mail]]

(In reply to Serge Gautherie from comment #28)
> (In reply to [e-mail] from comment #8)
> > 2. XPFE autocomplete does nothing if there is no text in the widget, unlike
> > toolkit autocomplete, which simply requests a search for the empty string.
> > (Search sessions can react in whichever way they feel appropriate.)
> 
> (In reply to [e-mail] from comment #10)
> > 3. XPFE autocomplete checks the results returned by the autocompletesearch
> > provider and bails if the search string doesn't match.
> 
> Neil, should we file a bug to change XPFE autocomplete behavior? (Or Toolkit
> one?)
Well, regarding empty searches, we would have to test very carefully our existing autocomplete implementations to ensure that they didn't get confused, for instance, I'm not sure what the address book would do. (Although if Thunderbird want to switch to toolkit they would have to deal with it anyway.)

> Or should we update test(s) that depend on Toolkit behavior?
Regarding ignoring of results that use the wrong search string, I'd have to ask the toolkit module owner what he thinks search sessions should do here.

Comment 30

[Serge Gautherie (:sgautherie)]

(In reply to [e-mail] from comment #26)
> What I meant is that only once LDAP has been switched to be a toolkit search
> session

I filed bug 733396, fwiw.

Comment 31

[Serge Gautherie (:sgautherie)]

(In reply to Serge Gautherie (:sgautherie) from comment #27)
> https://hg.mozilla.org/mozilla-central/rev/bcbdc2a0efa1

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331104973.1331111681.4909.gz&fulltext=1
Linux comm-central-trunk debug test mochitest-other on 2012/03/06 23:22:53

V.Fixed wrt this patch (only).

Comment 32

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:Jamie, could you have a look please?

For more information, please visit BugBot documentation.