Bug 121626

Summary: Spatial Navigation : User should be able to navigate html elements having element.style.cursor="pointer"
Product: WebKit Reporter: Abhijeet Kandalkar <kandalkar.abhijeet58>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED ---    
Severity: Normal CC: cmarcelo, commit-queue, cshu, esprehn+autocc, joepeck, kandalkar.abhijeet58, kangil.han, simon.fraser, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://jsfiddle.net/nsKjk/5/
Attachments:
Description Flags
Updated patch-1 tonikitoo: review-

Description Abhijeet Kandalkar 2013-09-19 12:15:20 PDT
Spatial Navigation: User should be able to navigate html elements having element.style.cursor="pointer"

 Using cursor properties is common practice in Web development.  Refer URL mentioned above; button is created using <span> which behaves like hyperlink. 

 As <span> is not focusable WebKit doesn’t draw focus ring. So to make <span> focusable element for spatial navigation, we need to adopt same approach mentioned in URL.
1.	If any HTML element has style.cursor="pointer", consider it as candidate for Spatial Navigation.
2.	If such element is chosen as best candidate, focus it by drawing border/outline around it.

@Antonio/Simon,
Please let me know your comments ?
Comment 1 Antonio Gomes 2013-09-19 13:07:39 PDT
> @Antonio/Simon,
> Please let me know your comments ?

Sounds good with me. Does Opera 12 (pre-blink) do similarly?
Comment 2 Abhijeet Kandalkar 2013-09-20 13:00:10 PDT
I checked Opera V12.14, Opera doesn't support this feature.
We should implement it in Webkit. 

Started working on it :)
Comment 3 Abhijeet Kandalkar 2013-10-03 08:00:13 PDT
Created attachment 213246 [details]
Updated patch-1
Comment 4 Antonio Gomes 2013-10-04 06:00:39 PDT
Comment on attachment 213246 [details]
Updated patch-1

View in context: https://bugs.webkit.org/attachment.cgi?id=213246&action=review

r-. See comments.


Also, a question: this property (cursor: point) seems inheritable. What happens if one does <body style="cursor:point"> ?

> Source/WebCore/dom/Element.cpp:251
> +bool Element::isSpatialNavigationFocusable() const
> +{
> +    if (!document().frame() && !document().frame()->settings().spatialNavigationEnabled())
> +        return false;
> +
> +    if (!inDocument())
> +        return false;
> +
> +    if (!renderer() || renderer()->style()->visibility() != VISIBLE)
> +        return false;
> +
> +    return (renderer()->style()->cursor() == CURSOR_POINTER);
> +}
> +

this does not belong to Element. See more below.

> Source/WebCore/page/FocusController.cpp:768
> +        if (!element->isSpatialNavigationFocusable() && !element->isKeyboardFocusable(event) && !element->isFrameOwnerElement() && !canScrollInDirection(element, direction))

isSpatialNavigationFocusable belong, I believe to SpatialNavigation.h, and should likely be a static method, taking an Element*

Additionally, it should be named with cursor:point case in mind. isSpatialNavigationFocusable is too generic in this case. Maybe hasCursorPointStyle?
Comment 5 Simon Fraser (smfr) 2013-10-07 12:49:46 PDT
Is this behavior described in a spec anywhere?
Comment 6 Abhijeet Kandalkar 2013-10-16 08:56:40 PDT
No, this behavior is not belongs to spec
This change will improve user interaction using spatial Navigation.

(In reply to comment #5)
> Is this behavior described in a spec anywhere?
Comment 7 Simon Fraser (smfr) 2013-10-16 09:07:06 PDT
(In reply to comment #6)
> No, this behavior is not belongs to spec
> This change will improve user interaction using spatial Navigation.

Then I don't think it should be implemented. If it's specified, then it's OK to implement. We want interoperability between browsers.
Comment 8 Abhijeet Kandalkar 2013-10-20 08:07:51 PDT
Hi Simon,

I agree with your point about interoperability between browsers but we are implementing this feature only if the spatial navigation is enabled. If user enabled spatial navigation then only this behavior is enabled for him.  I think this behavior will be helpful for user who wants to navigate WebPages using key navigation.
Moreover reasons to enable such behavior are,
1.	To make spatial navigation more user interactive.
2.	Opera browser already has this feature implemented. (Opera Version 12.14 using presto rendering engine) - http://www.opera.com/help/tutorials/nomouse/
3.	Most of web developers are using cursor pointer CSS style along with JavaScript event handlers, without abovementioned change user will not able to navigate such links.
<div style=”cursor: pointer” onclick=”navigateToUrl(‘www.google.com’)”> Click me</div>


(In reply to comment #7)
> (In reply to comment #6)
> > No, this behavior is not belongs to spec
> > This change will improve user interaction using spatial Navigation.
> 
> Then I don't think it should be implemented. If it's specified, then it's OK to implement. We want interoperability between browsers.
Comment 9 Abhijeet Kandalkar 2013-10-20 08:07:51 PDT
Hi Simon,

I agree with your point about interoperability between browsers but we are implementing this feature only if the spatial navigation is enabled. If user enabled spatial navigation then only this behavior is enabled for him.  I think this behavior will be helpful for user who wants to navigate WebPages using key navigation.
Moreover reasons to enable such behavior are,
1.	To make spatial navigation more user interactive.
2.	Opera browser already has this feature implemented. (Opera Version 12.14 using presto rendering engine) - http://www.opera.com/help/tutorials/nomouse/
3.	Most of web developers are using cursor pointer CSS style along with JavaScript event handlers, without abovementioned change user will not able to navigate such links.
<div style=”cursor: pointer” onclick=”navigateToUrl(‘www.google.com’)”> Click me</div>


(In reply to comment #7)
> (In reply to comment #6)
> > No, this behavior is not belongs to spec
> > This change will improve user interaction using spatial Navigation.
> 
> Then I don't think it should be implemented. If it's specified, then it's OK to implement. We want interoperability between browsers.