[webkit-reviews] review denied: [Bug 36463] Spatial Navigation: make it work with focusable elements in overflow content : [Attachment 56073] patch v4 - patch ready for review

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Antonio Gomes
(tonikitoo) <tonikitoo at webkit.org>'s request for review:
Bug 36463: Spatial Navigation: make it work with focusable elements in overflow
content
https://bugs.webkit.org/show_bug.cgi?id=36463

Attachment 56073: patch v4 - patch ready for review
https://bugs.webkit.org/attachment.cgi?id=56073&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
WebCore/page/FocusController.cpp:398
 +	    // sameContainerAsCandidate and sameContainerAsClosest are mutually
excluvive.
Typo: "excluvive"

WebCore/page/FocusController.cpp:405
 +	    } else if (sameContainerAsClosest) {
Don't use 'else' after a return.

WebCore/page/FocusController.cpp:409
 +	    } else { // ### !sameContainerAsCandidate &&
!sameContainerAsClosest
Ditto

WebCore/page/FocusController.cpp:449
 +	    } else if (candidate->hasTagName(divTag) &&
isScrollableContainerNode(candidate)) {
It's not correct to assume that only divs can have overflow: scroll. Any block
element can. It would be more appropriate to check the style for overflow.



r- for assuming that only divs can have overflow:scroll.