544171 - [SeaMonkey 2.1] mochitest-a11y: test_txtctrl.xul fails

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a1pre) Gecko/20100203 SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)
{
13710 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/tree/test_txtctrl.xul | Different amount of expected children of [ 'txc7' , role: autocomplete, name: 'hello']. - got 3, expected 2
}

Comment 1

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1265319973.1265321945.10606.gz
Linux comm-central-trunk debug test mochitest-other on 2010/02/04 13:46:13

Comment 2

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1283055773.1283058197.12882.gz&fulltext=1
Linux comm-central-trunk debug test mochitest-other on 2010/08/28 21:22:53

(Bug still there.)

Comment 3

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1300038585.1300041443.13407.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2011/03/13 10:49:45
{
19671 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tree/test_txtctrl.xul | Different amount of expected children of [ 'textbox@id='txc7' node' , role: autocomplete, name: 'hello', address: 0x9c6f740]. - got 3, expected 2
}

Comment 4

[Serge Gautherie (:sgautherie)]

With SeaMonkey, DOMi shows the following accessible tree:
*autocomplete (textbox)
 *entry (html:input)
  *text leaf (#text)
 *combobox list (xul:menupopup)
 *combobox list (xul:panel)
  *table (xul:tree)
   *list (xul:treecols)
    *columnheader (xul:treecol)
which is consistent with Neil's bug 535893 comment 1.

I'm not sure how I can install DOMi in Firefox so I could use it after running the test.
Anyway, iiuc looking at test source and comparing logs, the test doesn't expect the panel (and its children) to be there.

Is this acc.tree/difference expected for SeaMonkey?
If so, I should be able to adapt the test to support both autocomplete widgets.
(What would be the best way to check which widget is active? Instead of checking for "SeaMonkey" application.)

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> If so, I should be able to adapt the test to support both autocomplete widgets.
> (What would be the best way to check which widget is active? Instead of
> checking for "SeaMonkey" application.)

And would there be a way to force SeaMonkey to use the "Core"/bare autocomplete widget (for a specific textbox)?

Comment 6

[[e-mail]]

(In reply to comment #4)
> Is this acc.tree/difference expected for SeaMonkey?
Yes, SeaMonkey's autocomplete widget always includes an anonymous popup.
Toolkit's widget only creates one lazily if the autocompletepopup doesn't point
to an existing element. (Accessing the .popup field will trigger this, so this
might be one way of giving both autocompletes the same accessible tree.)

> (What would be the best way to check which widget is active? Instead of
> checking for "SeaMonkey" application.)
That's a good question. There are of course differences between the two
bindings, but some of them ought to be fixed for consistency. I think the best
one to test for is the clearResults method.

(In reply to comment #5)
> (In reply to comment #4)
> And would there be a way to force SeaMonkey to use the "Core"/bare autocomplete
> widget (for a specific textbox)?
You could explicitly specify the toolkit -moz-binding in its style.

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Document Toolkit autocomplete widget, Add support for XPFE autocomplete widget.

Thanks for your answers, Neil!

I decided that testing the Toolkit widget with Firefox only should be enough w.r.t this test.

I "documented" the Toolkit case, but I don't know how to make it work automatically :-|
Note that XPFE and Toolkit "popup" acc.tree are different (anyway).

Comment 8

[[e-mail]]

Comment on attachment 519065 [details] [diff] [review]
(Av1) Document Toolkit autocomplete widget, Add support for XPFE autocomplete widget.

>+// XXX (How) Can this be done without using alert()?
>+        alert(txc7.popup);
You could just assign it to a temporary variable, for instance.
Or maybe just writing txc7.popup; would work, but it looks odd!

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 519065 [details] [diff] [review]
(Av1) Document Toolkit autocomplete widget, Add support for XPFE autocomplete widget.


> 
>+      //
>       // password textbox
>+      //

nit: 80 of '/'

>+        // Popup is lazily created: trigger it.
>+// XXX (How) Can this be done without using alert()?
>+        alert(txc7.popup);

click it? you could use eventQueue and listen EVENT_REORDER to start a testing.

Comment 10

[Serge Gautherie (:sgautherie)]

Attachment: (Av2) Add support for XPFE (and Toolkit) autocomplete popup

Av1, with an added delay to support Toolkit case.


(In reply to comment #9)

> nit: 80 of '/'

I don't understand what you mean :-(

> >+// XXX (How) Can this be done without using alert()?
> 
> click it?

I eventually remembered why alert() makes a difference in such cases:
using a setTimeout() makes this work :-)

Comment 11

[alexander :surkov (:asurkov)]

(In reply to comment #10)
> Created attachment 519119 [details] [diff] [review]
> (Av2) Add support for XPFE (and Toolkit) autocomplete popup
> 
> Av1, with an added delay to support Toolkit case.
> 
> 
> (In reply to comment #9)
> 
> > nit: 80 of '/'
> 
> I don't understand what you mean :-(

I mean the comment like
/////////////////////////////////////// (80 '/' chars)
// comment
///////////////////////////////////////

we use this style usually :)

> > click it?
> 
> I eventually remembered why alert() makes a difference in such cases:
> using a setTimeout() makes this work :-)

does setTimeout() guarantee us it will work always (no timing failures)? Honestly I would prefer to use eventQueue here like

function alertInvoker()
{
  this.eventSeq = [ new invokerChecker(EVENT_REORDER, menupopupNode) ];
  this.invoke = function() { tx7.popup; }
  this.finalCheck = function() { checkTheTree(); }
  this.getID() = function() { return "description"; }
}
gQueue = new eventQueue();
gQueue.push(new alertInvoker());
gQueue.invoke(); // Will call SimpleTest.finish();

Comment 12

[Serge Gautherie (:sgautherie)]

Attachment: (WIP1) Try to use eventQueue() instead of setTimeout() [Moved to bug 642118]

(In reply to comment #11)

> > > nit: 80 of '/'

Ah, not 80 then, but 20 should be fine.

> does setTimeout() guarantee us it will work always (no timing failures)?

Not always, with a delay of '0', as I expected:
http://tbpl.mozilla.org/?tree=MozillaTry&rev=15d877c6e8c7
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1300107877.1300109085.23776.gz
Rev3 Fedora 12 tryserver opt test mochitest-other on 2011/03/14 06:04:37
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1300112132.1300113272.11865.gz
Rev3 Fedora 12x64 tryserver opt test mochitest-other on 2011/03/14 07:15:32

Yet, the 2 debug Linux passed, then an increased delay should hopefully be fine.

> Honestly I would prefer to use eventQueue here like

That would probably be better, but I'm unfamiliar with (writing) that.
This additional patch is what I (guessed and) tried, but it just never completes on my local Windows.
Could you check whether this is (hopefully) not written correctly, or it misses a delay still, or ...?

Maybe you could (want to) change this test to use eventQueue first, as you just did for bug 455840, then we would resume work on my patch(es)?

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 519153 [details] [diff] [review]
(WIP1) Try to use eventQueue() instead of setTimeout()
[Moved to bug 642118]

>+  this.finalCheck = function() { checkTheTree(); }

That should probably be
+  this.finalCheck = function() { testAccessibleTree("txc7", accTree); }
though it doesn't make a difference as is.

Comment 14

[alexander :surkov (:asurkov)]

(In reply to comment #13)
> Comment on attachment 519153 [details] [diff] [review]
> (WIP1) Try to use eventQueue() instead of setTimeout()

mostly copy-paste, but yes if that works then it's the way to go :)

> >+  this.finalCheck = function() { checkTheTree(); }
> 
> That should probably be
> +  this.finalCheck = function() { testAccessibleTree("txc7", accTree); }
> though it doesn't make a difference as is.

sure, testAccessibleTree

that was just an example, it should be styled, named, fitted properly though.

Comment 15

[Serge Gautherie (:sgautherie)]

Attachment: (Av2a) Add support for XPFE (and Toolkit) autocomplete popup [Checked in: Comment 19 & 24]

Av2, with more '/'s and a little delay.

Succeeded on
http://tbpl.mozilla.org/?tree=MozillaTry&rev=5c2303661ecd (Linux)
http://tbpl.mozilla.org/?tree=MozillaTry&rev=5c62ed506726 (Windows)
http://tbpl.mozilla.org/?tree=MozillaTry&rev=8280ac403c12 (Linux64)

NB: As discussed on irc, I'll then file a followup bug to investigate using an eventQueue.

Comment 16

[alexander :surkov (:asurkov)]

Comment on attachment 519462 [details] [diff] [review]
(Av2a) Add support for XPFE (and Toolkit) autocomplete popup
[Checked in: Comment 19 & 24]


>+      // Delay txc7 test a bit, to let Toolkit popup lazy creation complete.
>+      function test_txc7() {
>+        testAccessibleTree("txc7", accTree);
>+  
>+        SimpleTest.finish();
>+      }

>+      // SimpleTest.executeSoon() doesn't help here: use setTimeout() with a little delay.
>+      setTimeout(test_txc7, 25);

please file a bug to get rid timeout and refer to it here please

Comment 17

[[e-mail]]

Comment on attachment 519462 [details] [diff] [review]
(Av2a) Add support for XPFE (and Toolkit) autocomplete popup
[Checked in: Comment 19 & 24]

>+        // Dumb access to trigger popup lazy creation. (See code below.)
>+        txc7.popup;
I don't know whether it works for accessibility purposes but you might find that calling txc7.popup.getBoundingClientRect(); forces layout to update.

Comment 18

[alexander :surkov (:asurkov)]

(In reply to comment #17)
> Comment on attachment 519462 [details] [diff] [review]
> (Av2a) Add support for XPFE (and Toolkit) autocomplete popup.
> 
> >+        // Dumb access to trigger popup lazy creation. (See code below.)
> >+        txc7.popup;
> I don't know whether it works for accessibility purposes but you might find
> that calling txc7.popup.getBoundingClientRect(); forces layout to update.

not accessible tree though since layout RefreshDriver calls a11y async always iirc (that's when accessible tree creation happens).

Comment 19

[Serge Gautherie (:sgautherie)]

Comment on attachment 519462 [details] [diff] [review]
(Av2a) Add support for XPFE (and Toolkit) autocomplete popup
[Checked in: Comment 19 & 24]

http://hg.mozilla.org/mozilla-central/rev/4f07bebb993b


(In reply to comment #18)
> > that calling txc7.popup.getBoundingClientRect(); forces layout to update.
> 
> not accessible tree though since layout RefreshDriver calls a11y async always

I gave it a local try to confirm: it did not help.

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> please file a bug to get rid timeout and refer to it here please

I filed bug 642118.

Comment 21

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1300283071.1300284352.10711.gz&fulltext=1
Rev3 Fedora 12 mozilla-central opt test mochitest-other on 2011/03/16 06:44:31
{
19737 INFO TEST-PASS | chrome://mochitests/content/a11y/accessible/tree/test_txtctrl.xul | Testing (New) Toolkit autocomplete widget.
...
}

V.Fixed, Firefox part.

Comment 22

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1300301864.1300305152.29610.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2011/03/16 11:57:44
{
19682 INFO TEST-PASS | chrome://mochitests/content/a11y/accessible/tree/test_txtctrl.xul | Testing (Old) XPFE autocomplete widget.
}

V.Fixed, SeaMonkey part.

Comment 23

[Serge Gautherie (:sgautherie)]

Comment on attachment 519462 [details] [diff] [review]
(Av2a) Add support for XPFE (and Toolkit) autocomplete popup
[Checked in: Comment 19 & 24]

"approval2.0=?":
test-only, SM perma-orange fix.

Comment 24

[Serge Gautherie (:sgautherie)]

Comment on attachment 519462 [details] [diff] [review]
(Av2a) Add support for XPFE (and Toolkit) autocomplete popup
[Checked in: Comment 19 & 24]

http://hg.mozilla.org/releases/mozilla-2.0/rev/8e47e551c0bd