Open Bug 718237 Opened 8 years ago Updated 8 years ago

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

Categories

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

defect
Not set
major

Tracking

()

Tracking Status
firefox11 --- verified
firefox12 --- verified

People

(Reporter: sgautherie, Unassigned)

References

(Depends on 2 open bugs, Blocks 2 open bugs, )

Details

(Keywords: helpwanted, Whiteboard: [test disabled on SeaMonkey 2.8+] [fixed in mozilla11: Bv2; mozilla12: Av1; mozilla13: Cv1] [perma-orange] [qa-])

Attachments

(3 files, 1 obsolete file)

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.
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.
Depends on: 718239
[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.
Blocks: 673958
Keywords: helpwanted
Summary: [SeaMonkey] "accessible/events/test_focus_autocomplete.xul | Test timed out." → [SeaMonkey] "accessible/events/test_focus_autocomplete.xul | Test timed out." (which also causes lots of "gA11yEventListeners is undefined" on following tests)
Blocks: 706067, 467057
Depends on: 558589
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.
Attachment #588756 - Flags: review?(surkov.alexander)
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
Attachment #588756 - Flags: review?(surkov.alexander) → review+
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
Attachment #588756 - Attachment description: (Av1) accessible/tests/mochitest/*: Support "SeaMonkey searchbar" too, Remove 1 useless "browser.css" use → (Av1) accessible/tests/mochitest/*: Support "SeaMonkey searchbar" too, Remove 1 useless "browser.css" use [Checked in: Comment 5]
Whiteboard: [perma-orange] → [fixed in mozilla12: Av1] [perma-orange]
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.
Let's unbreak the whole(!) a11y suite, until we have a fix.
Attachment #598642 - Flags: review?(surkov.alexander)
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 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.
Attachment #598642 - Flags: review?(surkov.alexander) → review+
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.
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.
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.
Attachment #598642 - Attachment is obsolete: true
Attachment #598719 - Flags: review?(surkov.alexander)
Attachment #598719 - Flags: review?(surkov.alexander) → review+
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.
Attachment #598719 - Attachment description: (Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb → (Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb [Checked in: Comment 13]
Attachment #598719 - Flags: approval-mozilla-beta?
Attachment #598719 - Flags: approval-mozilla-aurora?
Whiteboard: [fixed in mozilla12: Av1] [perma-orange] → [fixed in mozilla12: Av1; mozilla13: Bv2] [perma-orange]
(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 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.
Attachment #598719 - Flags: approval-mozilla-beta?
Attachment #598719 - Flags: approval-mozilla-beta+
Attachment #598719 - Flags: approval-mozilla-aurora?
Attachment #598719 - Flags: approval-mozilla-aurora+
Keywords: checkin-needed
Whiteboard: [fixed in mozilla12: Av1; mozilla13: Bv2] [perma-orange] → [c-n: Bv2 to m-a and m-b] [fixed in mozilla12: Av1; mozilla13: Bv2] [perma-orange]
Jens, can you do this c-n too? (I/We miss it because this bug is not assigned to me...)
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
Attachment #598719 - Attachment description: (Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb [Checked in: Comment 13] → (Bv2) test_focus_autocomplete.xul: Skip this test on SeaMonkey ftb [Checked in: Comments 13 and 17]
Keywords: checkin-needed
Whiteboard: [c-n: Bv2 to m-a and m-b] [fixed in mozilla12: Av1; mozilla13: Bv2] [perma-orange] → [fixed in mozilla12: Av1; mozilla11: Bv2] [perma-orange]
Target Milestone: --- → mozilla13
(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.
Whiteboard: [fixed in mozilla12: Av1; mozilla11: Bv2] [perma-orange] → [test disabled on SeaMonkey] [fixed in mozilla11: Bv2; mozilla12: Av1] [perma-orange]
Whiteboard: [test disabled on SeaMonkey] [fixed in mozilla11: Bv2; mozilla12: Av1] [perma-orange] → [test disabled on SeaMonkey 2.8+] [fixed in mozilla11: Bv2; mozilla12: Av1] [perma-orange]
Set status to reflect Bv2 landing on branches.
Whiteboard: [test disabled on SeaMonkey 2.8+] [fixed in mozilla11: Bv2; mozilla12: Av1] [perma-orange] → [test disabled on SeaMonkey 2.8+] [fixed in mozilla11: Bv2; mozilla12: Av1] [perma-orange] [qa-]
Fixes comment 8 point 1 (only):
no more "search "test-a11y-search" not found - skipping." errors.
Attachment #603126 - Flags: review?(surkov.alexander)
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?
(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 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.
Attachment #603126 - Flags: review?(surkov.alexander) → review+
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.
(In reply to neil@parkwaycc.co.uk 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.
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 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
Attachment #603126 - Attachment description: (Cv1) Call initAutoComplete() early to support XPFE AutoComplete → (Cv1) Call initAutoComplete() early to support XPFE AutoComplete [Checked in: Comment 27]
Whiteboard: [test disabled on SeaMonkey 2.8+] [fixed in mozilla11: Bv2; mozilla12: Av1] [perma-orange] [qa-] → [test disabled on SeaMonkey 2.8+] [fixed in mozilla11: Bv2; mozilla12: Av1; mozilla13: Cv1] [perma-orange] [qa-]
Target Milestone: mozilla13 → ---
(In reply to neil@parkwaycc.co.uk 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 neil@parkwaycc.co.uk 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...
(In reply to Serge Gautherie from comment #28)
> (In reply to neil@parkwaycc.co.uk 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 neil@parkwaycc.co.uk 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.
(In reply to neil@parkwaycc.co.uk 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.
Depends on: 733396
(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).
You need to log in before you can comment on or make changes to this bug.