Closed Bug 439057 Opened 16 years ago Closed 16 years ago

Fix nsIAccessibleImage mochitest x and y coordinates so they work when running all tests

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.1a1

People

(Reporter: MarcoZ, Assigned: MarcoZ)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 3 obsolete files)

Address Surkov's comments
7.46 KB, patch
surkov
: review+
Details | Diff | Splinter Review 
Problem: The mochitests in accessible/test/mochitest/test_nsIAccessibleImage.html test whether we expose the x and y coordinates relative to the screen, window, and parent object. These differ from when the test page is run stand-alone from when the page is run in conjunction with other tests (clicking the "Run all tests" button at the front a11y harness page).

Question: Is there a way to get these coordinates also from JavaScript and simply compare them with the output of the a11y API call, or should I simply fix the values so they work when running all tests, but expectedly fail with the stand-alone test run?

Rob, Alex, any suggestions?
I just looked at Rob's error logs from bug 424195 again, and the values are different between operating systems, so hard-coding them is not an option.
(In reply to comment #1)
> I just looked at Rob's error logs from bug 424195 again, and the values are
> different between operating systems, so hard-coding them is not an option.
> 

true, on Mac there are many failures in stand-alone mode.
Attached patch Patch (obsolete) — — Splinter Review 
Make the tests for screen, window, and parent relative coordinates relative to one another. By this rule, screen coordinates must be greater than 0, window coordinates must be less than screen coordinates, and parent coordinates must be less than window coordinates.

This fixes the tests also when running in conjunction with other tests.
Assignee: nobody → marco.zehe
Status: NEW → ASSIGNED
Attachment #326852 - Flags: review?(surkov.alexander)
Blocks: 441974
Alexander, any chance you can get to this review soon?
sorry for the delay, just the code seems to me suspicious and I didn't get a time to run the patch. So below is what I get.

Move the window to the right to hide the biggest its part:

not ok - Wrong x screen coordinate for nonLinkedImage! got false, expected true
ok - Wrong y screen coordinate for nonLinkedImage!
not ok - window x coordinate must be less than screen coordinate for nonLinkedImage! got false, expected true
ok - window y coordinate must be less than screen coordinate for nonLinkedImage!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImage! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImage!
ok - Wrong width for nonLinkedImage!
ok - wrong height for nonLinkedImage!
ok - no attributes on nonLinkedImage!
ok - no correct src attribute for nonLinkedImage!
ok - No accessible for linkedImage!
ok - wrong name for linkedImage!
ok - no image interface for linkedImage!
not ok - Wrong x screen coordinate for linkedImage! got false, expected true
ok - Wrong y screen coordinate for linkedImage!
not ok - window x coordinate must be less than screen coordinate for linkedImage! got false, expected true
ok - window y coordinate must be less than screen coordinate for linkedImage!
ok - parent x coordinate must be less than window coordinate for linkedImage!
ok - parent y coordinate must be less than window coordinate for linkedImage!
ok - Wrong width for linkedImage!
ok - wrong height for linkedImage!
ok - no attributes on linkedImage!
ok - no correct src attribute for linkedImage!
ok - No accessible for nonLinkedImageWithAlt!
ok - wrong name for nonLinkedImageWithAlt!
ok - no image interface for nonLinkedImageWithAlt!
not ok - Wrong x screen coordinate for nonLinkedImageWithAlt! got false, expected true
ok - Wrong y screen coordinate for nonLinkedImageWithAlt!
not ok - window x coordinate must be less than screen coordinate for nonLinkedImageWithAlt! got false, expected true
ok - window y coordinate must be less than screen coordinate for nonLinkedImageWithAlt!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageWithAlt! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImageWithAlt!
ok - Wrong width for nonLinkedImageWithAlt!
ok - wrong height for nonLinkedImageWithAlt!
ok - no attributes on nonLinkedImageWithAlt!
ok - no correct src attribute for nonLinkedImageWithAlt!
ok - No accessible for linkedImageWithAlt!
ok - wrong name for linkedImageWithAlt!
ok - no image interface for linkedImageWithAlt!
not ok - Wrong x screen coordinate for linkedImageWithAlt! got false, expected true
ok - Wrong y screen coordinate for linkedImageWithAlt!
not ok - window x coordinate must be less than screen coordinate for linkedImageWithAlt! got false, expected true
ok - window y coordinate must be less than screen coordinate for linkedImageWithAlt!
ok - parent x coordinate must be less than window coordinate for linkedImageWithAlt!
ok - parent y coordinate must be less than window coordinate for linkedImageWithAlt!
ok - Wrong width for linkedImageWithAlt!
ok - wrong height for linkedImageWithAlt!
ok - no attributes on linkedImageWithAlt!
ok - no correct src attribute for linkedImageWithAlt!
ok - No accessible for nonLinkedImageWithTitle!
ok - wrong name for nonLinkedImageWithTitle!
ok - no image interface for nonLinkedImageWithTitle!
not ok - Wrong x screen coordinate for nonLinkedImageWithTitle! got false, expected true
ok - Wrong y screen coordinate for nonLinkedImageWithTitle!
not ok - window x coordinate must be less than screen coordinate for nonLinkedImageWithTitle! got false, expected true
ok - window y coordinate must be less than screen coordinate for nonLinkedImageWithTitle!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageWithTitle! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImageWithTitle!
ok - Wrong width for nonLinkedImageWithTitle!
ok - wrong height for nonLinkedImageWithTitle!
ok - no attributes on nonLinkedImageWithTitle!
ok - no correct src attribute for nonLinkedImageWithTitle!
ok - No accessible for linkedImageWithTitle!
ok - wrong name for linkedImageWithTitle!
ok - no image interface for linkedImageWithTitle!
not ok - Wrong x screen coordinate for linkedImageWithTitle! got false, expected true
ok - Wrong y screen coordinate for linkedImageWithTitle!
not ok - window x coordinate must be less than screen coordinate for linkedImageWithTitle! got false, expected true
ok - window y coordinate must be less than screen coordinate for linkedImageWithTitle!
ok - parent x coordinate must be less than window coordinate for linkedImageWithTitle!
ok - parent y coordinate must be less than window coordinate for linkedImageWithTitle!
ok - Wrong width for linkedImageWithTitle!
ok - wrong height for linkedImageWithTitle!
ok - no attributes on linkedImageWithTitle!
ok - no correct src attribute for linkedImageWithTitle!
ok - No accessible for nonLinkedImageEmptyAlt!
ok - wrong name for nonLinkedImageEmptyAlt!
ok - no image interface for nonLinkedImageEmptyAlt!
not ok - Wrong x screen coordinate for nonLinkedImageEmptyAlt! got false, expected true
ok - Wrong y screen coordinate for nonLinkedImageEmptyAlt!
not ok - window x coordinate must be less than screen coordinate for nonLinkedImageEmptyAlt! got false, expected true
ok - window y coordinate must be less than screen coordinate for nonLinkedImageEmptyAlt!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageEmptyAlt! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImageEmptyAlt!
ok - Wrong width for nonLinkedImageEmptyAlt!
ok - wrong height for nonLinkedImageEmptyAlt!
ok - no attributes on nonLinkedImageEmptyAlt!
ok - no correct src attribute for nonLinkedImageEmptyAlt!
ok - No accessible for linkedImageEmptyAlt!
ok - wrong name for linkedImageEmptyAlt!
ok - no image interface for linkedImageEmptyAlt!
not ok - Wrong x screen coordinate for linkedImageEmptyAlt! got false, expected true
ok - Wrong y screen coordinate for linkedImageEmptyAlt!
not ok - window x coordinate must be less than screen coordinate for linkedImageEmptyAlt! got false, expected true
ok - window y coordinate must be less than screen coordinate for linkedImageEmptyAlt!
ok - parent x coordinate must be less than window coordinate for linkedImageEmptyAlt!
ok - parent y coordinate must be less than window coordinate for linkedImageEmptyAlt!
ok - Wrong width for linkedImageEmptyAlt!
ok - wrong height for linkedImageEmptyAlt!
ok - no attributes on linkedImageEmptyAlt!
ok - no correct src attribute for linkedImageEmptyAlt!
ok - No accessible for nonLinkedImageEmptyAltAndTitle!
ok - wrong name for nonLinkedImageEmptyAltAndTitle!
ok - no image interface for nonLinkedImageEmptyAltAndTitle!
not ok - Wrong x screen coordinate for nonLinkedImageEmptyAltAndTitle! got false, expected true
ok - Wrong y screen coordinate for nonLinkedImageEmptyAltAndTitle!
not ok - window x coordinate must be less than screen coordinate for nonLinkedImageEmptyAltAndTitle! got false, expected true
ok - window y coordinate must be less than screen coordinate for nonLinkedImageEmptyAltAndTitle!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageEmptyAltAndTitle! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImageEmptyAltAndTitle!
ok - Wrong width for nonLinkedImageEmptyAltAndTitle!
ok - wrong height for nonLinkedImageEmptyAltAndTitle!
ok - no attributes on nonLinkedImageEmptyAltAndTitle!
ok - no correct src attribute for nonLinkedImageEmptyAltAndTitle!
ok - No accessible for linkedImageEmptyAltAndTitle!
ok - wrong name for linkedImageEmptyAltAndTitle!
ok - no image interface for linkedImageEmptyAltAndTitle!
not ok - Wrong x screen coordinate for linkedImageEmptyAltAndTitle! got false, expected true
ok - Wrong y screen coordinate for linkedImageEmptyAltAndTitle!
not ok - window x coordinate must be less than screen coordinate for linkedImageEmptyAltAndTitle! got false, expected true

When I run on Mac (don't move the window):

not ok - parent x coordinate must be less than window coordinate for nonLinkedImage! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImage!
ok - Wrong width for nonLinkedImage!
ok - wrong height for nonLinkedImage!
ok - no attributes on nonLinkedImage!
ok - no correct src attribute for nonLinkedImage!
ok - No accessible for linkedImage!
ok - wrong name for linkedImage!
ok - no image interface for linkedImage!
ok - Wrong x screen coordinate for linkedImage!
ok - Wrong y screen coordinate for linkedImage!
ok - window x coordinate must be less than screen coordinate for linkedImage!
ok - window y coordinate must be less than screen coordinate for linkedImage!
ok - parent x coordinate must be less than window coordinate for linkedImage!
ok - parent y coordinate must be less than window coordinate for linkedImage!
ok - Wrong width for linkedImage!
ok - wrong height for linkedImage!
ok - no attributes on linkedImage!
ok - no correct src attribute for linkedImage!
ok - No accessible for nonLinkedImageWithAlt!
ok - wrong name for nonLinkedImageWithAlt!
ok - no image interface for nonLinkedImageWithAlt!
ok - Wrong x screen coordinate for nonLinkedImageWithAlt!
ok - Wrong y screen coordinate for nonLinkedImageWithAlt!
ok - window x coordinate must be less than screen coordinate for nonLinkedImageWithAlt!
ok - window y coordinate must be less than screen coordinate for nonLinkedImageWithAlt!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageWithAlt! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImageWithAlt!
ok - Wrong width for nonLinkedImageWithAlt!
ok - wrong height for nonLinkedImageWithAlt!
ok - no attributes on nonLinkedImageWithAlt!
ok - no correct src attribute for nonLinkedImageWithAlt!
ok - No accessible for linkedImageWithAlt!
ok - wrong name for linkedImageWithAlt!
ok - no image interface for linkedImageWithAlt!
ok - Wrong x screen coordinate for linkedImageWithAlt!
ok - Wrong y screen coordinate for linkedImageWithAlt!
ok - window x coordinate must be less than screen coordinate for linkedImageWithAlt!
ok - window y coordinate must be less than screen coordinate for linkedImageWithAlt!
ok - parent x coordinate must be less than window coordinate for linkedImageWithAlt!
ok - parent y coordinate must be less than window coordinate for linkedImageWithAlt!
ok - Wrong width for linkedImageWithAlt!
ok - wrong height for linkedImageWithAlt!
ok - no attributes on linkedImageWithAlt!
ok - no correct src attribute for linkedImageWithAlt!
ok - No accessible for nonLinkedImageWithTitle!
ok - wrong name for nonLinkedImageWithTitle!
ok - no image interface for nonLinkedImageWithTitle!
ok - Wrong x screen coordinate for nonLinkedImageWithTitle!
ok - Wrong y screen coordinate for nonLinkedImageWithTitle!
ok - window x coordinate must be less than screen coordinate for nonLinkedImageWithTitle!
ok - window y coordinate must be less than screen coordinate for nonLinkedImageWithTitle!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageWithTitle! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImageWithTitle!
ok - Wrong width for nonLinkedImageWithTitle!
ok - wrong height for nonLinkedImageWithTitle!
ok - no attributes on nonLinkedImageWithTitle!
ok - no correct src attribute for nonLinkedImageWithTitle!
ok - No accessible for linkedImageWithTitle!
ok - wrong name for linkedImageWithTitle!
ok - no image interface for linkedImageWithTitle!
ok - Wrong x screen coordinate for linkedImageWithTitle!
ok - Wrong y screen coordinate for linkedImageWithTitle!
ok - window x coordinate must be less than screen coordinate for linkedImageWithTitle!
ok - window y coordinate must be less than screen coordinate for linkedImageWithTitle!
ok - parent x coordinate must be less than window coordinate for linkedImageWithTitle!
ok - parent y coordinate must be less than window coordinate for linkedImageWithTitle!
ok - Wrong width for linkedImageWithTitle!
ok - wrong height for linkedImageWithTitle!
ok - no attributes on linkedImageWithTitle!
ok - no correct src attribute for linkedImageWithTitle!
ok - No accessible for nonLinkedImageEmptyAlt!
ok - wrong name for nonLinkedImageEmptyAlt!
ok - no image interface for nonLinkedImageEmptyAlt!
ok - Wrong x screen coordinate for nonLinkedImageEmptyAlt!
ok - Wrong y screen coordinate for nonLinkedImageEmptyAlt!
ok - window x coordinate must be less than screen coordinate for nonLinkedImageEmptyAlt!
ok - window y coordinate must be less than screen coordinate for nonLinkedImageEmptyAlt!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageEmptyAlt! got false, expected true
ok - parent y coordinate must be less than window coordinate for nonLinkedImageEmptyAlt!
ok - Wrong width for nonLinkedImageEmptyAlt!
ok - wrong height for nonLinkedImageEmptyAlt!
ok - no attributes on nonLinkedImageEmptyAlt!
ok - no correct src attribute for nonLinkedImageEmptyAlt!
ok - No accessible for linkedImageEmptyAlt!
ok - wrong name for linkedImageEmptyAlt!
ok - no image interface for linkedImageEmptyAlt!
ok - Wrong x screen coordinate for linkedImageEmptyAlt!
ok - Wrong y screen coordinate for linkedImageEmptyAlt!
ok - window x coordinate must be less than screen coordinate for linkedImageEmptyAlt!
ok - window y coordinate must be less than screen coordinate for linkedImageEmptyAlt!
ok - parent x coordinate must be less than window coordinate for linkedImageEmptyAlt!
ok - parent y coordinate must be less than window coordinate for linkedImageEmptyAlt!
ok - Wrong width for linkedImageEmptyAlt!
ok - wrong height for linkedImageEmptyAlt!
ok - no attributes on linkedImageEmptyAlt!
ok - no correct src attribute for linkedImageEmptyAlt!
ok - No accessible for nonLinkedImageEmptyAltAndTitle!
ok - wrong name for nonLinkedImageEmptyAltAndTitle!
ok - no image interface for nonLinkedImageEmptyAltAndTitle!
ok - Wrong x screen coordinate for nonLinkedImageEmptyAltAndTitle!
ok - Wrong y screen coordinate for nonLinkedImageEmptyAltAndTitle!
ok - window x coordinate must be less than screen coordinate for nonLinkedImageEmptyAltAndTitle!
ok - window y coordinate must be less than screen coordinate for nonLinkedImageEmptyAltAndTitle!
not ok - parent x coordinate must be less than window coordinate for nonLinkedImageEmptyAltAndTitle! got false, expected true

Comment on attachment 326852 [details] [diff] [review]
Patch

clearing request
Attachment #326852 - Flags: review?(surkov.alexander)
Alex, do you have any other idea how to approach this?
I really don't know

1) Do not test coordinates :)
2) Use preprocessing macros to enable platform depend tests
3) Try to test relative coordinates
3.1) coordinates relative parent + parent coordinates relative window = coordinates relative window
3.2) get coordinates and pass them to getAccessibleFromPoint
Attached patch Different attempt (obsolete) — — Splinter Review 
1. Test whether parent x and y relative coordinates plus screen x and y coordinates of parent match the screen x and y coordinates of image. 
2. Note however, that for linked images, the parent-relative y coordinate on my system is always -27, and that the screen y coordinate of the image is actually the y screen coordinate of the parent link minus 1. This should be dealt with in a separate bug, and for now, I've just made sure that this test succeeds by hardcoding the image's parent y coordinate to -1 if it's negative in the first place.
3. Dropped tests for window relative x and y coordinates.

Alexander, does this work for you?
Attachment #326852 - Attachment is obsolete: true
Attachment #327929 - Flags: review?(surkov.alexander)
Comment on attachment 327929 [details] [diff] [review]
Different attempt


>+    function getCords(aImageAcc, aCordType)
>     {
>       var x = {}, y = {};
>       aImageAcc.getImagePosition(aCordType, x, y);
>-      is(x.value, aXCoordinate,
>-         "Wrong " + aCoordTypeString + " x offset for " + aID + "!");
>-      is(y.value, aYCoordinate,
>-         "Wrong " + aCoordTypeString + " y offset for " + aID + "!");
>+      gXCoordinate = x.value;
>+      gYCoordinate = y.value;

I really would prefer out arguments instead of global variables here.


>+      var xCoords = [0, 0, 0];
>+      var yCoords = [0, 0, 0];

I would like to use 6 variables instead of 2 arrays. That should be more readable.

You don't use window relative coordinates now. I would suggest to try
1) relative parent + parent relative screen = relative screen (that's you do)
2) relative window + window relative screen = relative screen
3) relative window = relative parent + parent relative window
4) test parent.getChildFromPoint( relative parent coordinates)

What do you think?

>-      testThis("linkedImageEmptyAltAndTitle", "Link to Mozilla Foundation", "moz.png", xCords, yCords, 93, 42);
>+      testThis("linkedImageEmptyAltAndTitle", "Link to Mozilla Foundation", "moz.png", 93, 42);

since you do relative tests then you shouldn't be forced to pass coordinates numbers.
Attached patch Next try (obsolete) — — Splinter Review 
I've got screen and parent relative coordinates working, but window relative coordinates still gives me headaches. I get the same x and y coordinates for the document accessible (role 16) and the XUL Document accessible a few ancestor steps above (role 75). However the x and y coordinates relative to window are always a much smaller number apart than the window boundary would suggest. On my system, for example, both screen x and window x, as well as screen y and window y, are both 4 pixels apart. The window's (or document's) bounds, however, are x=8 and y=114.

These tests are currently disabled, the rest passes.
Attachment #327929 - Attachment is obsolete: true
Attachment #327937 - Flags: review?(surkov.alexander)
Attachment #327929 - Flags: review?(surkov.alexander)
Comment on attachment 327937 [details] [diff] [review]
Next try


>+    function testCoordinates(aID, aAcc, aWidth, aHeight)
>     {

>+      if (imageAcc) {
>+        // get screen coordinates.

please use

if (!imageAcc)
  return;

that should make easier to read the patch.

>+//        var docAcc = null;
>+//        try {
>+//          docAcc = gAccRetrieval.getAccessibleFor(document);
>+//        } catch(e) {}
>+//        ok(docAcc, "No accessible for document with " + aID + "!");
>+

really, I would prefer to have one more bug with the wip patch instead of making this patch complicated. Commented code lines (or how to say this in english :)) is always bad.

more comments later
Removed tests that are currently not working. We can add them later again once we fix the API problems. The only exception is the y parent-relative coordinate for which I've left the constant -1 workaround in.
Attachment #327937 - Attachment is obsolete: true
Attachment #327951 - Flags: review?(surkov.alexander)
Attachment #327937 - Flags: review?(surkov.alexander)
Comment on attachment 327951 [details] [diff] [review]
Address Surkov's comments


>+      // For linked images, a negative parentY value is returned, and the
>+      // screenY coordinate is the link's screenY coordinate minus 1.
>+      // Until this is fixed, set parentY to -1 if it's negative.
>+      if (parentY.value < 0) {
>+        parentY.value = -1;
>+      }

please mark this section as XXX.
Attachment #327951 - Flags: review?(surkov.alexander) → review+
Pushed to mozilla-central in changeset:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/9da8390f7d46
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: