754830 - calculate link states separately

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

linkable accessible state relies on link states of the action accessible, having a way to receive link states without all states calculation should make it faster in 2 times.

Comment 1

[Trevor Saunders (:tbsaunde)]

Comment on attachment 623650 [details] [diff] [review]
patch

>+PRUint64
>+nsHTMLLinkAccessible::NativeLinkState() const
>+{
>+  nsEventStates eventState = mContent->AsElement()->State();
>+  if (eventState.HasAtLeastOneOfStates(NS_EVENT_STATE_VISITED |
>+                                       NS_EVENT_STATE_UNVISITED)) {
>+    PRUint64 state = states::LINKED;
>+    if (eventState.HasState(NS_EVENT_STATE_VISITED))
>+      state |= states::TRAVERSED;

it seems like it would be slightly faster and maybe clearer to check each condition seperately, so

if (states.HasState(unvisited))
  return states::linked;

if (states.HasState(visited))
  return states::linked | states::traversed

>+XULTextFieldAccessible::ApplyARIAState(PRUint64* aState) const
> {
>   nsHyperTextAccessibleWrap::ApplyARIAState(aState);
> 
>   aria::MapToState(aria::eARIAAutoComplete, mContent->AsElement(), aState);
> 
> }

nit, preexisting blank line

>+                                traversedLinkTester);
>+
>+      synthesizeMouse(getNode("link_traversed"), 1, 1, { shiftKey: true });
>     }
> 
>+    var traversedLinkTester = {
>+      handleEvent: function traversedLinkTester_handleEvent(aEvent) {
>+        unregisterA11yEventListener(EVENT_DOCUMENT_LOAD_COMPLETE,
>+                                    traversedLinkTester);
>+        aEvent.accessible.rootDocument.window.close();
>+
>+        testStates("link_traversed", STATE_TRAVERSED);
>+        SimpleTest.finish();
>+      }

it'd sort of be nice to test traversal state on more than just one type of link, but this is already an improvement on what we had so...

btw why this weird construct instead of a invoker / checker?

>      title="Expose click action if mouseup and mousedown are registered">
>     Mozilla Bug 423409
>   </a>
>   <p id="display"></p>
>   <div id="content" style="display: none"></div>

add bug info?

Comment 2

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #1)

> it'd sort of be nice to test traversal state on more than just one type of
> link, but this is already an improvement on what we had so...

yep, but what other types are?

> btw why this weird construct instead of a invoker / checker?

well, those eventQueue and waitForEvent stuffs need a target. I didn't want to calculate a target so did an easiest way. It sounds like one day we should finish with some API that allows that.

> add bug info?

it's not behavior change bug so we can skip it.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #2)

> it's not behavior change bug so we can skip it.

actually it changes ARIA behavior a bit. so I add a ref to the bug.

Comment 4

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb07c2f3427f

Comment 5

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

http://hg.mozilla.org/mozilla-central/rev/bb07c2f3427f

Comment 6

[Trevor Saunders (:tbsaunde)]

Comment on attachment 623650 [details] [diff] [review]
patch

>+  <a id="link_traversed" href="http://www.example.com" target="_top">example.com</a>

I somehow managed to not mention this before, but shouldn't you use a url that won't make us touch the network?

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #6)

> >+  <a id="link_traversed" href="http://www.example.com" target="_top">example.com</a>
> 
> I somehow managed to not mention this before, but shouldn't you use a url
> that won't make us touch the network?

this one is special for mochitest, it doesn't touch the network.