515133 - Intermittent failure of test_bug420863.html: td3: 'click' action hasn't been performed

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[(no longer active)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1252384967.1252389966.720.gz
OS X 10.5.2 mozilla-central unit test on 2009/09/07 21:42:47

640 INFO Running chrome://mochikit/content/a11y/accessible/test_bug420863.html...
641 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td1: shouldn't have actions
642 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td2: should have one action
643 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td2: should have 'click' action
644 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td2: 'click' action hasn't been performed
645 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td3: should have one action
646 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td3: should have 'click' action
647 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td3: 'click' action hasn't been performed
648 INFO TEST-KNOWN-FAIL | chrome://mochikit/content/a11y/accessible/test_bug420863.html | gAcc.numActions should not throw after click handler was removed!

Comment 1

[Marco Zehe (:MarcoZ)]

Taking.

Comment 2

[Marco Zehe (:MarcoZ)]

Attachment: WIP, doesn't work yet

This patch doesn't seem to work yet. It has 1 test failure in the test before the removal of the event listener, and all subsequent files give an error "nsIAccessibleEvent hasn't been defined" and other epic failures.

Alex, any idea what I'm doing wrong?

Comment 3

[alexander :surkov (:asurkov)]

I don't understand why you saved addEventListener stuffs. The problem can lie
in you call SimpleTest.finish() manually, however it is called by testActions
function. I think you fnished tests before they were testActions did the work
and therefore it didn't remove a11y event listeners (that's the reason of
nsIAccessibleEvent I think).

Comment 4

[Marco Zehe (:MarcoZ)]

Attachment: WIP2

With this, I get the following:

651 INFO Running chrome://mochikit/content/a11y/accessible/test_bug420863.html...
652 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | Simple table cell shouldn't have any actions
653 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | No actions on the accessible for  'td2' 
654 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | Wrong action name of the accessible for  'td2' 
655 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | test with ID = 'mousedown event handling' failed. No mousedown event.
656 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | test with ID = 'mouseup event handling' failed. No mouseup event.
657 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_bug420863.html | test with ID = 'click event handling' failed. No click event.
658 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_bug420863.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIAccessible.numActions]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mochikit/content/a11y/accessible/actions.js :: actionInvoker_invoke :: line 103"  data: no]

Alex, any idea why the accessible seems to be invalid for this call? I suspect it's trying to do something with td3Acc.

Comment 5

[alexander :surkov (:asurkov)]

I have no ideas on the glance, it's needed to debug I think.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

One thing: gClickHandler is not defined
getNode(this.ID).removeEventListener("click", gClickHandler, false);

I think you meant gTdClickEventHandler.

HTH

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

Or you need to wrap the gTdClickHandler with gClickHandler = { handleEvent:...

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

The other thing that can confound this is that doAction is essentially asynchronous. So when we call myAcc.doAction(0) our C++ code eventually ends up in nsAccessible::DoCommand where we use a one shot timer for the actual click.

This should be quick, but maybe js wins sometimes (perhaps layout is busy) and we get ahead of ourselves?

I actually like the original test, but maybe try increasing the timer delay in:
      gAcc.doAction(0);

      // actions are performed via timeout
      window.setTimeout(doTest3, 0);

Perhaps window.setTimeout(doTest3, 50); would suffice.

Comment 9

[Marco Zehe (:MarcoZ)]

Attachment: Patch

Got it working! Had to fix a typo (see David's comment above), and also had to keepe the click handler as a global var. I also had to pass the node into the array instead of the acc, and now it works.

Comment 10

[alexander :surkov (:asurkov)]

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


> 
>       // register 'click' event handler
>       gClickHandler = {
>         handleEvent: function handleEvent(aEvent)
>         {
>           gTdClickEventHandler = true;
>         }
>       };

>+          ID: td3Node,
>+          actionName: "click",
>+          actionIndex: 0,
>+          events: CLICK_EVENTS,
>+          checkOnClickEvent: function check(aEvent)
>+          {
>+            ok(gTdClickEventHandler,
>+               "td3: 'click' action hasn't been performed");

it doesn't make sense to use and check gTdClickEventHandler variable because checkOnClickEvent means click event was handled already.


>+            // check actions
>+            // XXX see bug 456347, sometimes after removing the event listener, the
>+            // accessible is no longer valid.
>+            var numActions = -1;
>+            try {
>+              numActions = getAccessible(this.ID).numActions;
>+            } catch(e) {}
> 
>-      // unregister click event handler
>-      gNode.removeEventListener("click", gClickHandler, false);
>+            if (numActions == -1)
>+              todo(false,
>+                   "gAcc.numActions should not throw after click handler was removed!");
>+            else
>+              is(numActions, 0, "td3 shouldn't have actions");
>+          }

bug 456347 is worksforme, so we don't need this code. Just check numActions.

Comment 11

[Marco Zehe (:MarcoZ)]

Attachment: Patch2, addresses Surkov's comments

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 399693 [details] [diff] [review]
Patch2, addresses Surkov's comments


>+          checkOnClickEvent: function check(aEvent)
>+          {
>+            ok(true,
>+               "td3: 'click' action hasn't been performed");

this ok is not necessary as well because there is ok inside of evenQueue when click is handled, otherwise should ok.

Comment 13

[Marco Zehe (:MarcoZ)]

Pushed with comment addressed: http://hg.mozilla.org/mozilla-central/rev/0a71e9c0cdec

Comment 14

[Legacy TBPL/Treeherder Robot]

dholbert%mozilla.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6/1281018583.1281019372.19065.gz
WINNT 5.2 mozilla-1.9.2 opt test mochitest-other on 2010/08/05 07:29:43

s: win32-slave36
576 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_bug420863.html | td3: 'click' action hasn't been performed