calculate link states separately

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 2 bugs, {access})

unspecified
mozilla15
access
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 623650 [details] [diff] [review]
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.
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+
(Assignee)

Comment 2

5 years ago
(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.
(Assignee)

Comment 3

5 years ago
(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.
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb07c2f3427f
Target Milestone: --- → mozilla15
http://hg.mozilla.org/mozilla-central/rev/bb07c2f3427f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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?
(Assignee)

Comment 7

5 years ago
(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.