415730 - convert existing API tests into mochitests

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Bernd]

http://www.mozilla.org/quality/embed/plans/accessibility/HTMLAccessibility.html

contains tests which are by its nature very similar to mochitests. We should convert them, bring them into the tree and make them part of the tinderbox testing.

alexander surkov in bug 410052
"We can add mochitest but we can't enable this because some another mochitests
become broken with enabled accessibility. I think we'll take care of this after
Firefox3."

There needs to be a bug reference for this to get this solved before.

Comment 1

[Bernd]

this are the really good ones
http://www.mozilla.org/projects/ui/accessibility/unix/testcase/nsIAccExt/TestCases_for_HTML_Elements.html

Comment 2

[Marco Zehe (:MarcoZ)]

Assigning this to myself since this is going to be my baby. As stated in bug 410052, I will find a way to have ally Mochitests run after all others are run, so that when a11y gets enabled, it doesn#t interfere with Mochitests that depend on being run without.

Comment 3

[alexander :surkov (:asurkov)]

Marco, please keep in mind to enable http://lxr.mozilla.org/mozilla/source/layout/xul/test/test_bug368835.xul test

Comment 4

[alexander :surkov (:asurkov)]

remember about bug 410052 to add test for it

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> Assigning this to myself since this is going to be my baby. As stated in bug
> 410052, I will find a way to have ally Mochitests run after all others are run,
> so that when a11y gets enabled, it doesn#t interfere with Mochitests that
> depend on being run without.
> 

Not sure it's correct way to run our mochitests after all tests because mochitests shouldn't fail in anyway when ally is enabled.

Comment 6

[Rob Campbell [:rc] (:robcee)]

I think we should do something similar to the way we're running browser-chrome and chrome tests. Provide a separate option to runtests.pl|y (--a11y ?) that runs tests from a separate location. We might want to keep them isolated.

Also, and this is probably a separate bug, but Martijn suggested last week that we should consider setting up a machine/profile to run mochitests with accessibility turned on to see what breaks.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> I think we should do something similar to the way we're running browser-chrome
> and chrome tests. Provide a separate option to runtests.pl|y (--a11y ?) that
> runs tests from a separate location. We might want to keep them isolated.

Ok, I'll try this.

> Also, and this is probably a separate bug, but Martijn suggested last week that
> we should consider setting up a machine/profile to run mochitests with
> accessibility turned on to see what breaks.
> 

It would be excellent.

Comment 8

[Bernd]

Attachment: mochitest for the attributes of nsIAccessibleTable

Comment 9

[Bernd]

the columnHeader is bug 417797
the rwoHeader    is bug 417791

Comment 10

[Bernd]

Just to avoid duplicates, I will write a complete set for nsIAccessibleTable.

Comment 11

[Bernd]

Attachment: nsIAccessibeTable mochitest

Comment 12

[Bernd]

This is basically my test for all things that the idl file lists. The many todos prove my point from https://bugzilla.mozilla.org/show_bug.cgi?id=410052#c37 

Comment 13

[Marco Zehe (:MarcoZ)]

Comment on attachment 303769 [details] [diff] [review]
nsIAccessibeTable mochitest

> Index: test_bug410052_1.html
...
> +   //columnHeaderIndex =columnHeader.getIndexAt(0,2);
> +   //is(columnHeaderIndex, 2, "columnheaderindex is wrong");

Did you rmean to remove these or add them back in at some point?

r=me with the above explained/fixed.

Comment 14

[alexander :surkov (:asurkov)]

files have wrong code style and I'd appretiate to use more readable file names than bug name because it's actually common tests. Bernd, Marco could you fix this, I guess we don't need approval to land this because it's tests only?

Comment 15

[Bernd]

>files have wrong code style
I don't know how to fix this as this is very vague.

Comment 16

[Bernd]

+    todo(works, "columnHeader should not throw");
+   //columnHeaderIndex =columnHeader.getIndexAt(0,2);
+   //is(columnHeaderIndex, 2, "columnheaderindex is wrong");

Currently the code throws, so the tests should be only added once the thing behaves as expected.

Comment 17

[Bernd]

s/files have wrong code style/should follow the http://developer.mozilla.org/en/docs/JavaScript_style_guide/ ?

Comment 18

[alexander :surkov (:asurkov)]

Comment on attachment 303769 [details] [diff] [review]
nsIAccessibeTable mochitest

I'll show some examples:

>+   var caption = accNode.caption;   
>+   is(caption.firstChild.name, "Test Table", "wrong text inside caption");
>+   const nsIAccess = Components.interfaces.nsIAccessible;

I would like to call it nsIAccessNode

>+   is(caption.children.queryElementAt(0, nsIAccess).name, "Test Table", "wrong text inside caption");

you shouldn't exceed 80 chars restriction.

>+   var range = document.createRange();
>+    range.selectNode(cell);

wrong indentation

>+   try
>+   {
>+     rowheader  = accNode.rowHeader;

iirc should be
try {
}

>+   }
>+   catch(e){

no whitespace between ( and }

>+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=410052">Mozilla Bug 410052</a>
>+  <p id="display"></p>

wrong inentation

>+  
>+</body></html>
>\ No newline at end of file

no new line :)

>+      var accNode = accService.getAccessibleFor(table).
>+      QueryInterface(Components.interfaces.nsIAccessibleTable);
>+
wron indentation
  


>+   if(s.rangeCount > 0) s.removeAllRanges();

should be 
if (s.rangeCount > 0)
  s.removeAllRanges();

Comment 19

[Bernd]

Attachment: revised patch

Comment 20

[Bernd]

I checked the testcases with the comments from Alexander in.

Comment 21

[Marco Zehe (:MarcoZ)]

Closing INVALID, since these tests don't exist any more and we probably have coverage by now.

Comment 22

[alexander :surkov (:asurkov)]

(In reply to Marco Zehe (:MarcoZ) from comment #21)
> Closing INVALID, since these tests don't exist any more and we probably have
> coverage by now.

these tests were renamed and actually this bug is fixed, changeset is http://hg.mozilla.org/mozilla-central/rev/9002bf7b7346