[Webkit-unassigned] [Bug 116046] For keyboard users, activating a fragment URL should transfer focus and caret to the destination

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=116046

--- Comment #33 from Nan Wang <n_wang at apple.com> ---
Comment on attachment 276287
  --> https://bugs.webkit.org/attachment.cgi?id=276287
patch

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

>> Source/WebCore/dom/Document.cpp:3969
>> +        Node* nextNode = NodeTraversal::nextSkippingChildren(*node);
> 
> Why are we calling nextSkippingChildren here given we're moving to the container node in nodeChildrenWillBeRemoved already?

Because in nodeWillBeRemoved we are moving to the previousSibling if possible. I think running ElementTraversal::previous on  nextSkippingChildren will cover both cases.
I just found a bug with this in such case: <div id="container"><input id="removed"><input><input></div>
After removing first input, next focused element will be the last input but we want the second one. I think we should use next instead of nextSkippingChildren.

>> Source/WebCore/dom/Document.cpp:4101
>> +        if (m_focusNavigationStartingNode.get() == nodeToBeRemoved) {
> 
> This is a very inefficient way of checking that m_focusNavigationStartingNode is getting removed.
> Instead, just check "m_focusNavigationStartingNode && m_focusNavigationStartingNode->parentNode() == this".
> Also, this code has a bug that it doesn't check the possibility that an ancestor of m_focusNavigationStartingNode is getting removed.
> e.g. m_focusNavigationStartingNode is at the span in <div><span></span></div> and div is getting removed.
> So we need to walk up the ancestor chain of m_focusNavigationStartingNode and check whether any of it matches this.
> r- because of this bug.
> Also, please add a test case for this.

Good point, will handle this case.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160425/eb610c20/attachment-0001.html>