Last Comment Bug 754830 - calculate link states separately
: calculate link states separately
Status: RESOLVED FIXED
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: alexander :surkov
:
:
Mentors:
Depends on:
Blocks: a11yperf 732872
  Show dependency treegraph
 
Reported: 2012-05-14 06:18 PDT by alexander :surkov
Modified: 2012-05-17 20:10 PDT (History)
2 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (22.28 KB, patch)
2012-05-14 06:18 PDT, alexander :surkov
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description alexander :surkov 2012-05-14 06:18:04 PDT
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.
Comment 1 Trevor Saunders (:tbsaunde) 2012-05-16 11:30:02 PDT
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 2012-05-17 00:12:32 PDT
(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 2012-05-17 00:14:46 PDT
(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 5 Ryan VanderMeulen [:RyanVM] 2012-05-17 11:18:32 PDT
http://hg.mozilla.org/mozilla-central/rev/bb07c2f3427f
Comment 6 Trevor Saunders (:tbsaunde) 2012-05-17 13:34:40 PDT
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 2012-05-17 20:10:23 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.