Last Comment Bug 760451 - [AccessFu] Support skip links
: [AccessFu] Support skip links
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla15
Assigned To: Eitan Isaacson [:eeejay]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-01 07:49 PDT by Eitan Isaacson [:eeejay]
Modified: 2012-06-05 12:03 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Support skip links (2.05 KB, patch)
2012-06-01 08:18 PDT, Eitan Isaacson [:eeejay]
dbolter: review+
Details | Diff | Splinter Review

Description Eitan Isaacson [:eeejay] 2012-06-01 07:49:00 PDT
Sites will often have links that skip to anchors in the site. We should move the virtual cursor to the anchor.
Comment 1 Eitan Isaacson [:eeejay] 2012-06-01 08:18:48 PDT
Created attachment 629198 [details] [diff] [review]
Support skip links

This uses EVENT_SCROLLING_START, which is a curious name, but seems to do what we need.
Comment 2 David Bolter [:davidb] 2012-06-01 08:41:24 PDT
Comment on attachment 629198 [details] [diff] [review]
Support skip links

Review of attachment 629198 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/jsat/AccessFu.jsm
@@ +351,5 @@
>          }
>          break;
>        }
> +      case Ci.nsIAccessibleEvent.EVENT_SCROLLING_START:
> +      {

Yeah, strange but true - this seems to be the event to watch. I wonder why we didn't call it something related to anchors...

::: accessible/src/jsat/VirtualCursorController.jsm
@@ +174,5 @@
> +      } catch (x) {
> +        doc = doc.parentDocument;
> +        continue;
> +      }
> +      vc.moveNext(aRule || this.SimpleTraversalRule, aAccessible, true);

Will this throw a type error if vc is null? (Can vc be null here?)
Comment 3 Eitan Isaacson [:eeejay] 2012-06-01 08:46:07 PDT
(In reply to David Bolter [:davidb] from comment #2)
> Comment on attachment 629198 [details] [diff] [review]
> Support skip links
> 
> Review of attachment 629198 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/jsat/VirtualCursorController.jsm
> @@ +174,5 @@
> > +      } catch (x) {
> > +        doc = doc.parentDocument;
> > +        continue;
> > +      }
> > +      vc.moveNext(aRule || this.SimpleTraversalRule, aAccessible, true);
> 
> Will this throw a type error if vc is null? (Can vc be null here?)

If it has the cursorable interface, it should always have a virtual cursor. But null checking would not be such a bad practice here, so I could add that.
Comment 4 David Bolter [:davidb] 2012-06-01 08:47:44 PDT
Comment on attachment 629198 [details] [diff] [review]
Support skip links

Review of attachment 629198 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the null check thanks.
Comment 5 David Bolter [:davidb] 2012-06-01 08:48:46 PDT
Wait, there are no cases it wouldn't have a vc? If so then no null check is fine.
Comment 6 Eitan Isaacson [:eeejay] 2012-06-03 19:05:40 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/48a60f4f7264
Comment 8 Marco Zehe (:MarcoZ) on PTO until August 15 2012-06-05 12:03:49 PDT
Verified fixed in Fennec/Android 16.0a1 2012/06/05.

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