Closed Bug 754830 Opened 8 years ago Closed 8 years ago

calculate link states separately

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 2 open bugs)

Details

(Keywords: access)

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #623650 - Flags: review?(trev.saunders)
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?
Attachment #623650 - Flags: review?(trev.saunders) → review+
(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.
(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.
http://hg.mozilla.org/mozilla-central/rev/bb07c2f3427f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
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?
(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.
You need to log in before you can comment on or make changes to this bug.