Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sequential focus navigation #13460

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -83,6 +83,7 @@ use dom::touchevent::TouchEvent;
use dom::touchlist::TouchList;
use dom::treewalker::TreeWalker;
use dom::uievent::UIEvent;
use dom::virtualmethods::VirtualMethods;
use dom::webglcontextevent::WebGLContextEvent;
use dom::window::{ReflowReason, Window};
use encoding::EncodingRef;
@@ -238,6 +239,9 @@ pub struct Document {
dom_complete: Cell<u64>,
load_event_start: Cell<u64>,
load_event_end: Cell<u64>,
/// Vector to store focusable elements
focus_list: DOMRefCell<Vec<JS<Element>>>,
focus_list_index: Cell<usize>,
/// https://html.spec.whatwg.org/multipage/#concept-document-https-state
https_state: Cell<HttpsState>,
touchpad_pressure_phase: Cell<TouchpadPressurePhase>,
@@ -496,6 +500,33 @@ impl Document {
}
}

/// Add a focusable element to this document's ordering focus list.
pub fn add_focusable_element(&self, element: &Element) {
self.focus_list.borrow_mut().push(JS::from_ref(element));
}

/// Removes a focusable element from this document's ordering focus list.
pub fn remove_focusable_element(&self, element: &Element) {
if self.focus_list.borrow().len() > 0 {

let mut index = 0;

for item in self.focus_list.borrow().iter() {
if &**item == element {
break;
}

index += 1;
}

This comment has been minimized.

Copy link
@nox

nox Jan 20, 2017

Member

You can use Iterator::position for this.


let mut list = self.focus_list.borrow_mut();

if index < list.len() {
list.remove(index);
}
}
}

/// Associate an element present in this document with the provided id.
pub fn register_named_element(&self, element: &Element, id: Atom) {
debug!("Adding named element to document {:p}: {:p} id={}",
@@ -1176,6 +1207,9 @@ impl Document {
event.fire(target);
let mut prevented = event.DefaultPrevented();

self.handle_event(event);
self.commit_focus_transaction(FocusType::Element);

// https://w3c.github.io/uievents/#keys-cancelable-keys
if state != KeyState::Released && props.is_printable() && !prevented {
// https://w3c.github.io/uievents/#keypress-event-order
@@ -1656,6 +1690,63 @@ impl Document {

self.window.layout().nodes_from_point(page_point, *client_point)
}

// https://html.spec.whatwg.org/multipage/#sequential-focus-navigation
fn sequential_focus_navigation(&self, event: &KeyboardEvent) {
// Step 1
// Step 2
// TODO: Implement the sequential focus navigation starting point.
// This can be used to change the starting point for navigation.

let modifier = event.get_key_modifiers();

// Step 3
let direction =
if modifier.is_empty() {
NavigationDirection::Forward
} else {
NavigationDirection::Backward
};

// TODO: Step 4

// Step 5
let candidate = self.sequential_search(direction);

// Step 6
self.request_focus(&candidate);
}

// https://html.spec.whatwg.org/multipage/interaction.html#sequential-navigation-search-algorithm
// TODO: Use the starting point and selection mechanism for searching
fn sequential_search(&self, direction: NavigationDirection) -> Root<Element> {
let list = self.focus_list.borrow();
let ref current_index = self.focus_list_index;
let focus_state = list[current_index.get()].focus_state();

if focus_state {
match direction {
NavigationDirection::Forward => {
if current_index.get() != list.len() - 1 {
current_index.set(current_index.get() + 1);
}
else {
current_index.set(0);
}
},
NavigationDirection::Backward => {
if current_index.get() != 0 {
current_index.set(current_index.get() - 1);
}
else {
current_index.set(list.len() - 1);
}
},
}
}

Root::from_ref(&*list[current_index.get()])
}
}

#[derive(PartialEq, HeapSizeOf)]
@@ -1799,6 +1890,8 @@ impl Document {
dom_complete: Cell::new(Default::default()),
load_event_start: Cell::new(Default::default()),
load_event_end: Cell::new(Default::default()),
focus_list: DOMRefCell::new(vec![]),
focus_list_index: Cell::new(0),
https_state: Cell::new(HttpsState::None),
touchpad_pressure_phase: Cell::new(TouchpadPressurePhase::BeforeClick),
origin: origin,
@@ -1975,6 +2068,31 @@ impl Document {
}
}

impl VirtualMethods for Document {
fn super_type(&self) -> Option<&VirtualMethods> {
Some(self.upcast::<Document>() as &VirtualMethods)
}

fn handle_event(&self, event: &Event) {
if let Some(key_event) = event.downcast::<KeyboardEvent>() {
if event.type_() == atom!("keydown") {
let key = key_event.get_key().unwrap();

match key {
Key::Tab => {
self.sequential_focus_navigation(key_event);
},
_ => (),
}
}
}
}
}

enum NavigationDirection {
Forward,
Backward
}

impl Element {
fn click_event_filter_by_disabled_state(&self) -> bool {
@@ -2203,6 +2203,19 @@ impl VirtualMethods for Element {
return;
}

if self.is_focusable_area() {
let doc = document_from_node(self);
let children = self.upcast::<Node>().traverse_preorder();

for child in children {
if let Some(element) = child.downcast::<Element>() {
if element.is_focusable_area() && !element.disabled_state() {
doc.add_focusable_element(element);
}
}
}
}

This comment has been minimized.

Copy link
@nox

nox Oct 17, 2016

Member

When do you remove focusable elements from the document? Shouldn't that be done in unbind_from_tree?


if let Some(ref value) = *self.id_attribute.borrow() {
let doc = document_from_node(self);
doc.register_named_element(self, value.clone());
@@ -2216,6 +2229,9 @@ impl VirtualMethods for Element {
return;
}

let doc = document_from_node(self);
doc.remove_focusable_element(self);

if let Some(ref value) = *self.id_attribute.borrow() {
let doc = document_from_node(self);
doc.unregister_named_element(self, value.clone());
@@ -20,11 +20,12 @@ use dom::event::Event;
use dom::eventtarget::EventTarget;
use dom::htmlelement::HTMLElement;
use dom::htmlimageelement::HTMLImageElement;
use dom::keyboardevent::KeyboardEvent;
use dom::mouseevent::MouseEvent;
use dom::node::{Node, document_from_node, window_from_node};
use dom::urlhelper::UrlHelper;
use dom::virtualmethods::VirtualMethods;
use msg::constellation_msg::ReferrerPolicy;
use msg::constellation_msg::{Key, ReferrerPolicy};
use num_traits::ToPrimitive;
use script_traits::MozBrowserEvent;
use std::default::Default;
@@ -99,6 +100,23 @@ impl VirtualMethods for HTMLAnchorElement {
_ => self.super_type().unwrap().parse_plain_attribute(name, value),
}
}

fn handle_event(&self, event: &Event) {

This comment has been minimized.

Copy link
@nox

nox Oct 17, 2016

Member

You didn't call the super type handle_event method.

if let Some(s) = self.super_type() {
s.handle_event(event);
}

if let Some(key_event) = event.downcast::<KeyboardEvent>() {
if event.type_() == atom!("keydown") {
match key_event.get_key() {
Some(Key::Enter) => {
follow_hyperlink(self.upcast::<Element>(), Some(String::new()), None);
},
_ => ()
}
}
}
}
}

impl HTMLAnchorElementMethods for HTMLAnchorElement {
@@ -559,7 +577,7 @@ fn is_current_browsing_context(target: DOMString) -> bool {
}

/// https://html.spec.whatwg.org/multipage/#following-hyperlinks-2
fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option<String>, referrer_policy: Option<ReferrerPolicy>) {
pub fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option<String>, referrer_policy: Option<ReferrerPolicy>) {

This comment has been minimized.

Copy link
@nox

nox Jan 20, 2017

Member

AFAIK this doesn't need to be public.

// Step 1: replace.
// Step 2: source browsing context.
// Step 3: target browsing context.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.