[webkit-reviews] review denied: [Bug 22740] It's not possible for clients of QWebFrame to implement directional focus node navigation : [Attachment 25852] patch to QWebFrame to get the next focus point given a point and a direction

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <tavestbo at trolltech.com> has denied Joseph Ligman
<joseph.ligman at nokia.com>'s request for review:
Bug 22740: It's not possible for clients of QWebFrame to implement directional
focus node navigation
https://bugs.webkit.org/show_bug.cgi?id=22740

Attachment 25852: patch to QWebFrame to get the next focus point given a point
and a direction
https://bugs.webkit.org/attachment.cgi?id=25852&action=review

------- Additional Comments from Tor Arne Vestbø <tavestbo at trolltech.com>
Good stuff Joe, but I have a few questions about the concept, let's discuss on
IRC.

Some style comments, see http://webkit.org/coding/coding-style.html and
http://trac.webkit.org/wiki/Coding%20Style%20Guidelines

> +    //The dotDist is the euclidian distance

Spaces beteeen // and comment

> + int dotDist = 0;

A better name for this?

> + int Overlap = 0;
> + int xdisplacement = 0;
> + int ydisplacement = 0;

Variables should be lowerCaseTitleCase


> +	  dy = focus.y()-rect.bottom();

Spaces between operators

> +    }
> +    else if (direction == QWebFrame::South) {

else if on the same line as closing brace

> +	   else if (focus.y() > rect.bottom())
> +	       dy = focus.y() - rect.bottom();
> +
> +    }

Don't need the blank line

> +	   //overlapping rectangles are rewarded
> +	   if ((rect.y()>=rbound.y() && rect.y()<=rbound.bottom()) ||
> +	       (rect.bottom()>=rbound.y() && rect.bottom()<=rbound.bottom())) {


Boolean operators aligned on the left side, not on the right
> +    QPoint next =
m_view->page()->mainFrame()->nextFocusPoint(QWebFrame::South,current);

Can probably store mainFrame here and use later instead of going through the
view and page.