Closed Bug 544171 Opened 10 years ago Closed 9 years ago

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

Categories

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

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla5
Tracking Status
status2.0 --- ?

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(Blocks 3 open bugs, )

Details

(Whiteboard: [perma-orange])

Attachments

(1 file, 3 obsolete files)

[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
}
Keywords: access
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
OS: Windows 2000 → All
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.)
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
}
Depends on: 535893
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.)
(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)?
(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.
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).
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #519065 - Flags: review?(surkov.alexander)
Attachment #519065 - Flags: feedback?(neil)
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!
Attachment #519065 - Flags: feedback?(neil) → feedback+
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.
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 :-)
Attachment #519065 - Attachment is obsolete: true
Attachment #519119 - Flags: review?(surkov.alexander)
Attachment #519065 - Flags: review?(surkov.alexander)
(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();
(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)?
Attachment #519153 - Flags: feedback?(surkov.alexander)
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.
(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.
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.
Attachment #519119 - Attachment is obsolete: true
Attachment #519462 - Flags: review?(surkov.alexander)
Attachment #519119 - Flags: review?(surkov.alexander)
Attachment #519462 - Attachment filename: 544171-Av2_popup-support.diff → 544171-Av2a_popup-support.diff
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
Attachment #519462 - Flags: review?(surkov.alexander) → review+
Attachment #519153 - Flags: feedback?(surkov.alexander)
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.
(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 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.
Attachment #519462 - Attachment description: (Av2a) Add support for XPFE (and Toolkit) autocomplete popup. → (Av2a) Add support for XPFE (and Toolkit) autocomplete popup [Checked in: Comment 19]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Keywords: access
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
Blocks: 642118
Attachment #519153 - Attachment description: (WIP1) Try to use eventQueue() instead of setTimeout() → (WIP1) Try to use eventQueue() instead of setTimeout() [Moved to bug 642118]
Attachment #519153 - Attachment is obsolete: true
(In reply to comment #16)
> please file a bug to get rid timeout and refer to it here please

I filed bug 642118.
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.
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.
Status: RESOLVED → VERIFIED
Whiteboard: [perma-orange]
Target Milestone: mozilla2.0 → mozilla2.2
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.
Attachment #519462 - Flags: approval2.0?
Attachment #519462 - Flags: approval2.0? → approval2.0+
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
Attachment #519462 - Attachment description: (Av2a) Add support for XPFE (and Toolkit) autocomplete popup [Checked in: Comment 19] → (Av2a) Add support for XPFE (and Toolkit) autocomplete popup [Checked in: Comment 19 & 24]
status2.0: --- → ?
You need to log in before you can comment on or make changes to this bug.