Commit 3568f132 authored by yael.aharon@nokia.com's avatar yael.aharon@nokia.com

Navigating downwards / upwards does not focus on the links spread across more than one line.

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

Reviewed by Antonio Gomes.

Source/WebCore: 

When 2 anchor elements span more than one line each, and one ends on the same line that the
second starts on, the rects reported by their renderers are overlapping. When handling
2 overlapping nodes, check for this case, and don't assume that one of the nodes is on a higher layer.   

Test: fast/spatial-navigation/snav-two-elements-one-line.html

* page/FocusController.cpp:
(WebCore::updateFocusCandidateIfNeeded):
(WebCore::FocusController::findFocusCandidateInContainer):
* page/SpatialNavigation.cpp:
(WebCore::areElementsOnSameLine):
(WebCore::distanceDataForNode):
* page/SpatialNavigation.h:

LayoutTests: 

* fast/spatial-navigation/snav-two-elements-one-line-expected.txt: Added.
* fast/spatial-navigation/snav-two-elements-one-line.html: Added.



git-svn-id: svn://svn.chromium.org/blink/trunk@79021 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 379486fa
2011-02-17 Yael Aharon <yael.aharon@nokia.com>
Reviewed by Antonio Gomes.
Navigating downwards / upwards does not focus on the links spread across more than one line.
https://bugs.webkit.org/show_bug.cgi?id=54639
* fast/spatial-navigation/snav-two-elements-one-line-expected.txt: Added.
* fast/spatial-navigation/snav-two-elements-one-line.html: Added.
2011-02-18 Andrew Wilson <atwilson@chromium.org> 2011-02-18 Andrew Wilson <atwilson@chromium.org>
Unreviewed, rolling out r79007. Unreviewed, rolling out r79007.
......
AAAAAAAAAA AAAAAAAAAA AA AA AAAAAAAAAA AAAAAAAAAA
AAAAAAAAAA AAAAAAAAAA AA B AA AAAAAAAAAA AAAAAAAAAA
PASS gFocusedDocument.activeElement.getAttribute("id") is "e1"
PASS gFocusedDocument.activeElement.getAttribute("id") is "e2"
PASS gFocusedDocument.activeElement.getAttribute("id") is "e3"
PASS gFocusedDocument.activeElement.getAttribute("id") is "e2"
PASS gFocusedDocument.activeElement.getAttribute("id") is "e1"
PASS gFocusedDocument.activeElement.getAttribute("id") is "start"
This test is testing that we can handle 2 inline elements in the same line.
<html>
<head>
<script src="../js/resources/js-test-pre.js"></script>
<script src="resources/spatial-navigation-utils.js"></script>
<script type="application/javascript">
var resultMap = [
["Down", "e1"],
["Down", "e2"],
["Down", "e3"],
["Up", "e2"],
["Up", "e1"],
["Up", "start"],
["DONE", "DONE"]
];
if (window.layoutTestController) {
layoutTestController.dumpAsText();
layoutTestController.setSpatialNavigationEnabled(true);
layoutTestController.overridePreference("WebKitTabToLinksPreferenceKey", 1);
layoutTestController.waitUntilDone();
}
function runTest()
{
// starting the test itself: get to a known place.
document.getElementById("start").focus();
initTest(resultMap, testCompleted);
}
function testCompleted()
{
if (window.layoutTestController)
layoutTestController.notifyDone();
}
window.onload = runTest;
</script>
<script src="js/resources/js-test-post.js"></script>
<style>
div.container1 { width: 100px;}
div.container2 { width: 100px;}
</style>
</head>
<body id="some-content" xmlns="http://www.w3.org/1999/xhtml" style="padding:20px">
<div class="container1">
<a href="#" id="start">AAAAAAAAAA AAAAAAAAAA AA</a>
<a href="#" id="e1">AA AAAAAAAAAA AAAAAAAAAA</a>
</div>
<br><br>
<div class="container2">
<a href="#" id="e2">AAAAAAAAAA AAAAAAAAAA AA</a>
<span>B</span>
<a href="#" id="e3">AA AAAAAAAAAA AAAAAAAAAA</a>
</div>
<div id="console"></div>
This test is testing that we can handle 2 inline elements in the same line.
</body>
</html>
2011-02-18 Yael Aharon <yael.aharon@nokia.com>
Reviewed by Antonio Gomes.
Navigating downwards / upwards does not focus on the links spread across more than one line.
https://bugs.webkit.org/show_bug.cgi?id=54639
When 2 anchor elements span more than one line each, and one ends on the same line that the
second starts on, the rects reported by their renderers are overlapping. When handling
2 overlapping nodes, check for this case, and don't assume that one of the nodes is on a higher layer.
Test: fast/spatial-navigation/snav-two-elements-one-line.html
* page/FocusController.cpp:
(WebCore::updateFocusCandidateIfNeeded):
(WebCore::FocusController::findFocusCandidateInContainer):
* page/SpatialNavigation.cpp:
(WebCore::areElementsOnSameLine):
(WebCore::distanceDataForNode):
* page/SpatialNavigation.h:
2011-02-18 Ben Vanik <benvanik@google.com> 2011-02-18 Ben Vanik <benvanik@google.com>
Reviewed by Kenneth Russell. Reviewed by Kenneth Russell.
......
...@@ -421,7 +421,7 @@ void FocusController::setActive(bool active) ...@@ -421,7 +421,7 @@ void FocusController::setActive(bool active)
dispatchEventsOnWindowAndFocusedNode(m_focusedFrame->document(), active); dispatchEventsOnWindowAndFocusedNode(m_focusedFrame->document(), active);
} }
static void updateFocusCandidateIfNeeded(FocusDirection direction, const IntRect& startingRect, FocusCandidate& candidate, FocusCandidate& closest) static void updateFocusCandidateIfNeeded(FocusDirection direction, const FocusCandidate& current, FocusCandidate& candidate, FocusCandidate& closest)
{ {
ASSERT(candidate.visibleNode->isElementNode()); ASSERT(candidate.visibleNode->isElementNode());
ASSERT(candidate.visibleNode->renderer()); ASSERT(candidate.visibleNode->renderer());
...@@ -434,8 +434,6 @@ static void updateFocusCandidateIfNeeded(FocusDirection direction, const IntRect ...@@ -434,8 +434,6 @@ static void updateFocusCandidateIfNeeded(FocusDirection direction, const IntRect
if (candidate.isOffscreen && !canBeScrolledIntoView(direction, candidate)) if (candidate.isOffscreen && !canBeScrolledIntoView(direction, candidate))
return; return;
FocusCandidate current;
current.rect = startingRect;
distanceDataForNode(direction, current, candidate); distanceDataForNode(direction, current, candidate);
if (candidate.distance == maxDistance()) if (candidate.distance == maxDistance())
return; return;
...@@ -449,7 +447,7 @@ static void updateFocusCandidateIfNeeded(FocusDirection direction, const IntRect ...@@ -449,7 +447,7 @@ static void updateFocusCandidateIfNeeded(FocusDirection direction, const IntRect
} }
IntRect intersectionRect = intersection(candidate.rect, closest.rect); IntRect intersectionRect = intersection(candidate.rect, closest.rect);
if (!intersectionRect.isEmpty()) { if (!intersectionRect.isEmpty() && !areElementsOnSameLine(closest, candidate)) {
// If 2 nodes are intersecting, do hit test to find which node in on top. // If 2 nodes are intersecting, do hit test to find which node in on top.
int x = intersectionRect.x() + intersectionRect.width() / 2; int x = intersectionRect.x() + intersectionRect.width() / 2;
int y = intersectionRect.y() + intersectionRect.height() / 2; int y = intersectionRect.y() + intersectionRect.height() / 2;
...@@ -478,6 +476,11 @@ void FocusController::findFocusCandidateInContainer(Node* container, const IntRe ...@@ -478,6 +476,11 @@ void FocusController::findFocusCandidateInContainer(Node* container, const IntRe
Node* focusedNode = (focusedFrame() && focusedFrame()->document()) ? focusedFrame()->document()->focusedNode() : 0; Node* focusedNode = (focusedFrame() && focusedFrame()->document()) ? focusedFrame()->document()->focusedNode() : 0;
Node* node = container->firstChild(); Node* node = container->firstChild();
FocusCandidate current;
current.rect = startingRect;
current.focusableNode = focusedNode;
current.visibleNode = focusedNode;
for (; node; node = (node->isFrameOwnerElement() || canScrollInDirection(node, direction)) ? node->traverseNextSibling(container) : node->traverseNextNode(container)) { for (; node; node = (node->isFrameOwnerElement() || canScrollInDirection(node, direction)) ? node->traverseNextSibling(container) : node->traverseNextNode(container)) {
if (node == focusedNode) if (node == focusedNode)
continue; continue;
...@@ -493,7 +496,7 @@ void FocusController::findFocusCandidateInContainer(Node* container, const IntRe ...@@ -493,7 +496,7 @@ void FocusController::findFocusCandidateInContainer(Node* container, const IntRe
continue; continue;
candidate.enclosingScrollableBox = container; candidate.enclosingScrollableBox = container;
updateFocusCandidateIfNeeded(direction, startingRect, candidate, closest); updateFocusCandidateIfNeeded(direction, current, candidate, closest);
} }
} }
......
...@@ -39,6 +39,7 @@ ...@@ -39,6 +39,7 @@
#include "IntRect.h" #include "IntRect.h"
#include "Node.h" #include "Node.h"
#include "Page.h" #include "Page.h"
#include "RenderInline.h"
#include "RenderLayer.h" #include "RenderLayer.h"
#include "Settings.h" #include "Settings.h"
...@@ -594,12 +595,39 @@ void entryAndExitPointsForDirection(FocusDirection direction, const IntRect& sta ...@@ -594,12 +595,39 @@ void entryAndExitPointsForDirection(FocusDirection direction, const IntRect& sta
} }
} }
bool areElementsOnSameLine(const FocusCandidate& firstCandidate, const FocusCandidate& secondCandidate)
{
if (firstCandidate.isNull() || secondCandidate.isNull())
return false;
if (!firstCandidate.visibleNode->renderer() || !secondCandidate.visibleNode->renderer())
return false;
if (!firstCandidate.rect.intersects(secondCandidate.rect))
return false;
if (firstCandidate.focusableNode->hasTagName(HTMLNames::areaTag) || secondCandidate.focusableNode->hasTagName(HTMLNames::areaTag))
return false;
if (!firstCandidate.visibleNode->renderer()->isRenderInline() || !secondCandidate.visibleNode->renderer()->isRenderInline())
return false;
if (firstCandidate.visibleNode->renderer()->containingBlock() != secondCandidate.visibleNode->renderer()->containingBlock())
return false;
return true;
}
void distanceDataForNode(FocusDirection direction, const FocusCandidate& current, FocusCandidate& candidate) void distanceDataForNode(FocusDirection direction, const FocusCandidate& current, FocusCandidate& candidate)
{ {
if (candidate.isNull()) if (areElementsOnSameLine(current, candidate)) {
return; if ((direction == FocusDirectionUp && current.rect.y() > candidate.rect.y()) || (direction == FocusDirectionDown && candidate.rect.y() > current.rect.y())) {
if (!candidate.visibleNode->renderer()) candidate.distance = 0;
return; candidate.alignment = Full;
return;
}
}
IntRect nodeRect = candidate.rect; IntRect nodeRect = candidate.rect;
IntRect currentRect = current.rect; IntRect currentRect = current.rect;
deflateIfOverlapped(currentRect, nodeRect); deflateIfOverlapped(currentRect, nodeRect);
......
...@@ -141,6 +141,7 @@ bool scrollInDirection(Node* container, FocusDirection); ...@@ -141,6 +141,7 @@ bool scrollInDirection(Node* container, FocusDirection);
bool canScrollInDirection(const Node* container, FocusDirection); bool canScrollInDirection(const Node* container, FocusDirection);
bool canScrollInDirection(const Frame*, FocusDirection); bool canScrollInDirection(const Frame*, FocusDirection);
bool canBeScrolledIntoView(FocusDirection, const FocusCandidate&); bool canBeScrolledIntoView(FocusDirection, const FocusCandidate&);
bool areElementsOnSameLine(const FocusCandidate& firstCandidate, const FocusCandidate& secondCandidate);
void distanceDataForNode(FocusDirection, const FocusCandidate& current, FocusCandidate& candidate); void distanceDataForNode(FocusDirection, const FocusCandidate& current, FocusCandidate& candidate);
Node* scrollableEnclosingBoxOrParentFrameForNodeInDirection(FocusDirection, Node*); Node* scrollableEnclosingBoxOrParentFrameForNodeInDirection(FocusDirection, Node*);
IntRect nodeRectInAbsoluteCoordinates(Node*, bool ignoreBorder = false); IntRect nodeRectInAbsoluteCoordinates(Node*, bool ignoreBorder = false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment