Diff
Modified: trunk/LayoutTests/ChangeLog (153703 => 153704)
--- trunk/LayoutTests/ChangeLog 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/LayoutTests/ChangeLog 2013-08-05 13:09:19 UTC (rev 153704)
@@ -1,3 +1,15 @@
+2013-08-05 Abhijeet Kandalkar <[email protected]>
+
+ Spatial Navigation should avoid unwanted calculation while deciding focus candidate.
+ https://bugs.webkit.org/show_bug.cgi?id=117265
+
+ Reviewed by Antonio Gomes.
+
+ Added testcases to count how many target nodes were tested before choosing a final target.
+
+ * fast/spatial-navigation/snav-search-optimization-expected.txt: Added.
+ * fast/spatial-navigation/snav-search-optimization.html: Added.
+
2013-08-05 Mihai Tica <[email protected]>
[CSS Background Blending] Specifying background-image and background-color with opaque
Added: trunk/LayoutTests/fast/spatial-navigation/snav-search-optimization-expected.txt (0 => 153704)
--- trunk/LayoutTests/fast/spatial-navigation/snav-search-optimization-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/spatial-navigation/snav-search-optimization-expected.txt 2013-08-05 13:09:19 UTC (rev 153704)
@@ -0,0 +1,13 @@
+1 2 3
+4
+ 6
+7 8 9
+PASS internals.lastSpatialNavigationCandidateCount() is 9
+PASS internals.lastSpatialNavigationCandidateCount() is 6
+PASS internals.lastSpatialNavigationCandidateCount() is 2
+PASS internals.lastSpatialNavigationCandidateCount() is 9
+PASS internals.lastSpatialNavigationCandidateCount() is 6
+PASS internals.lastSpatialNavigationCandidateCount() is 2
+PASS internals.lastSpatialNavigationCandidateCount() is 5
+PASS internals.lastSpatialNavigationCandidateCount() is 2
+
Added: trunk/LayoutTests/fast/spatial-navigation/snav-search-optimization.html (0 => 153704)
--- trunk/LayoutTests/fast/spatial-navigation/snav-search-optimization.html (rev 0)
+++ trunk/LayoutTests/fast/spatial-navigation/snav-search-optimization.html 2013-08-05 13:09:19 UTC (rev 153704)
@@ -0,0 +1,101 @@
+<html>
+ <!--
+ This test ensures the optimization done in searching logic to find best candidate focusable node with minimum iterations.
+
+ * Pre-conditions:
+ 1) TestRunner support for SNav enable/disable.
+
+ * Navigation steps:
+ 1) Loads this page, focus goes to "2".
+ 2) lastSpatialNavigationCandidateCount() returns the number of nodes actually considered to determine best candidate focusable node,
+ along the most recent navigation direction.
+ -->
+<head>
+ <script src=""
+ <script src=""
+ <script type="application/_javascript_">
+
+ var resultMap = [
+ ["DONE", "DONE"]
+ ];
+
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setSpatialNavigationEnabled(true);
+ testRunner.overridePreference("WebKitTabToLinksPreferenceKey", 1);
+ testRunner.waitUntilDone();
+ }
+
+ function runTest()
+ {
+ initTest(resultMap, additionalTest);
+ }
+
+ function additionalTest()
+ {
+ document.getElementById("2").focus();
+ eventSender.keyDown("downArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "9");
+ eventSender.keyDown("downArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "6");
+ eventSender.keyDown("downArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "2");
+ eventSender.keyDown("rightArrow");
+ eventSender.keyDown("upArrow");
+ eventSender.keyDown("leftArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "9");
+ eventSender.keyDown("leftArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "6");
+ eventSender.keyDown("leftArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "2");
+ eventSender.keyDown("upArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "5");
+ eventSender.keyDown("upArrow");
+ shouldBe("internals.lastSpatialNavigationCandidateCount()", "2");
+ testCompleted();
+ }
+
+ function testCompleted()
+ {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+
+ window._onload_ = runTest;
+
+ </script>
+ <script src=""
+</head>
+<body id="some-content" xmlns="http://www.w3.org/1999/xhtml">
+ <table style="text-align: left; margin-left: auto; margin-right: auto;" border="1" cellpadding="2" cellspacing="1">
+ <tbody>
+ <tr>
+ <td style="vertical-align: top; text-align: center;"><a id="1" href=""
+ <td style="vertical-align: top; text-align: center;"><a id="2" href=""
+ <td style="vertical-align: top; text-align: center;"><a id="3" href=""
+ </tr>
+ <tr>
+ <td style="text-align: center;"><a id="4" href=""
+ <td><br><iframe width='100' height='100' src=""
+ <table style='text-align: center; margin-left: auto; margin-right: auto;'>
+ <tbody>
+ <tr>
+ <td style='text-align: center;'><a id='5' href=''>5</a></td>
+ </tr>
+ </tbody>
+ </table>">
+ </iframe>
+ </td>
+ <td style="text-align: center;"><a id="6" href=""
+ </tr>
+ <tr>
+ <td style="vertical-align: top; text-align: center;"><a id="7" href=""
+ <td style="vertical-align: top; text-align: center;"><a id="8" href=""
+ <td style="vertical-align: top; text-align: center;"><a id="9" href=""
+ </tr>
+ </tbody>
+ </table>
+ <div id="console"></div>
+</body>
+</html>
+
Modified: trunk/Source/WebCore/ChangeLog (153703 => 153704)
--- trunk/Source/WebCore/ChangeLog 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/ChangeLog 2013-08-05 13:09:19 UTC (rev 153704)
@@ -1,3 +1,32 @@
+2013-08-05 Abhijeet Kandalkar <[email protected]>
+
+ Spatial Navigation should avoid unwanted calculation while deciding focus candidate.
+ https://bugs.webkit.org/show_bug.cgi?id=117265
+
+ Reviewed by Antonio Gomes.
+
+ Spatial Navigation should consider only those nodes as candidate which are exactly in the focus-direction.
+ e.g. If we are moving down then the nodes that are above CURRENT focused node should be considered as invalid.
+ Added isValidCandidate() which checks whether node is exactly in the focus-direction.
+
+ Test: fast/spatial-navigation/snav-search-optimization.html
+
+ * page/FocusController.cpp:
+ (WebCore::FocusController::findFocusCandidateInContainer):
+ (WebCore::FocusController::advanceFocusDirectionally):
+ * page/Page.cpp:
+ (WebCore::Page::Page):
+ * page/Page.h:
+ (WebCore::Page::setLastSpatialNavigationCandidateCount):
+ (WebCore::Page::lastSpatialNavigationCandidateCount):
+ * page/SpatialNavigation.cpp:
+ (WebCore::isValidCandidate):
+ * page/SpatialNavigation.h:
+ * testing/Internals.cpp:
+ (WebCore::Internals::lastSpatialNavigationCandidateCount):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2013-08-05 Mihai Tica <[email protected]>
[CSS Background Blending] Specifying background-image and background-color
Modified: trunk/Source/WebCore/page/FocusController.cpp (153703 => 153704)
--- trunk/Source/WebCore/page/FocusController.cpp 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/page/FocusController.cpp 2013-08-05 13:09:19 UTC (rev 153704)
@@ -766,6 +766,7 @@
current.focusableNode = focusedNode;
current.visibleNode = focusedNode;
+ unsigned candidateCount = 0;
for (; element; element = (element->isFrameOwnerElement() || canScrollInDirection(element, direction))
? ElementTraversal::nextSkippingChildren(element, container)
: ElementTraversal::next(element, container)) {
@@ -779,9 +780,20 @@
if (candidate.isNull())
continue;
+ if (!isValidCandidate(direction, current, candidate))
+ continue;
+
+ candidateCount++;
candidate.enclosingScrollableBox = container;
updateFocusCandidateIfNeeded(direction, current, candidate, closest);
}
+
+ // The variable 'candidateCount' keeps track of the number of nodes traversed in a given container.
+ // If we have more than one container in a page then the total number of nodes traversed is equal to the sum of nodes traversed in each container.
+ if (focusedFrame() && focusedFrame()->document()) {
+ candidateCount += focusedFrame()->document()->page()->lastSpatialNavigationCandidateCount();
+ focusedFrame()->document()->page()->setLastSpatialNavigationCandidateCount(candidateCount);
+ }
}
bool FocusController::advanceFocusDirectionallyInContainer(Node* container, const LayoutRect& startingRect, FocusDirection direction, KeyboardEvent* event)
@@ -868,7 +880,7 @@
if (container->isDocumentNode())
toDocument(container)->updateLayoutIgnorePendingStylesheets();
-
+
// Figure out the starting rect.
LayoutRect startingRect;
if (focusedElement) {
@@ -882,6 +894,9 @@
}
}
+ if (focusedFrame() && focusedFrame()->document())
+ focusedDocument->page()->setLastSpatialNavigationCandidateCount(0);
+
bool consumed = false;
do {
consumed = advanceFocusDirectionallyInContainer(container, startingRect, direction, event);
Modified: trunk/Source/WebCore/page/Page.cpp (153703 => 153704)
--- trunk/Source/WebCore/page/Page.cpp 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/page/Page.cpp 2013-08-05 13:09:19 UTC (rev 153704)
@@ -187,6 +187,7 @@
, m_scriptedAnimationsSuspended(false)
, m_pageThrottler(PageThrottler::create(this))
, m_console(PageConsole::create(this))
+ , m_lastSpatialNavigationCandidatesCount(0) // NOTE: Only called from Internals for Spatial Navigation testing.
, m_framesHandlingBeforeUnloadEvent(0)
{
ASSERT(m_editorClient);
Modified: trunk/Source/WebCore/page/Page.h (153703 => 153704)
--- trunk/Source/WebCore/page/Page.h 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/page/Page.h 2013-08-05 13:09:19 UTC (rev 153704)
@@ -409,6 +409,8 @@
void incrementFrameHandlingBeforeUnloadEventCount();
void decrementFrameHandlingBeforeUnloadEventCount();
bool isAnyFrameHandlingBeforeUnloadEvent();
+ void setLastSpatialNavigationCandidateCount(unsigned count) { m_lastSpatialNavigationCandidatesCount = count; }
+ unsigned lastSpatialNavigationCandidateCount() const { return m_lastSpatialNavigationCandidatesCount; }
private:
void initGroup();
@@ -546,7 +548,8 @@
HashSet<String> m_seenPlugins;
HashSet<String> m_seenMediaEngines;
-
+
+ unsigned m_lastSpatialNavigationCandidatesCount;
unsigned m_framesHandlingBeforeUnloadEvent;
};
Modified: trunk/Source/WebCore/page/SpatialNavigation.cpp (153703 => 153704)
--- trunk/Source/WebCore/page/SpatialNavigation.cpp 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/page/SpatialNavigation.cpp 2013-08-05 13:09:19 UTC (rev 153704)
@@ -620,6 +620,28 @@
return true;
}
+// Consider only those nodes as candidate which are exactly in the focus-direction.
+// e.g. If we are moving down then the nodes that are above current focused node should be considered as invalid.
+bool isValidCandidate(FocusDirection direction, const FocusCandidate& current, FocusCandidate& candidate)
+{
+ LayoutRect currentRect = current.rect;
+ LayoutRect candidateRect = candidate.rect;
+
+ switch (direction) {
+ case FocusDirectionLeft:
+ return candidateRect.x() < currentRect.maxX();
+ case FocusDirectionUp:
+ return candidateRect.y() < currentRect.maxY();
+ case FocusDirectionRight:
+ return candidateRect.maxX() > currentRect.x();
+ case FocusDirectionDown:
+ return candidateRect.maxY() > currentRect.y();
+ default:
+ ASSERT_NOT_REACHED();
+ }
+ return false;
+}
+
void distanceDataForNode(FocusDirection direction, const FocusCandidate& current, FocusCandidate& candidate)
{
if (areElementsOnSameLine(current, candidate)) {
Modified: trunk/Source/WebCore/page/SpatialNavigation.h (153703 => 153704)
--- trunk/Source/WebCore/page/SpatialNavigation.h 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/page/SpatialNavigation.h 2013-08-05 13:09:19 UTC (rev 153704)
@@ -136,6 +136,7 @@
bool canScrollInDirection(const Frame*, FocusDirection);
bool canBeScrolledIntoView(FocusDirection, const FocusCandidate&);
bool areElementsOnSameLine(const FocusCandidate& firstCandidate, const FocusCandidate& secondCandidate);
+bool isValidCandidate(FocusDirection, const FocusCandidate&, FocusCandidate&);
void distanceDataForNode(FocusDirection, const FocusCandidate& current, FocusCandidate& candidate);
Node* scrollableEnclosingBoxOrParentFrameForNodeInDirection(FocusDirection, Node*);
LayoutRect nodeRectInAbsoluteCoordinates(Node*, bool ignoreBorder = false);
Modified: trunk/Source/WebCore/testing/Internals.cpp (153703 => 153704)
--- trunk/Source/WebCore/testing/Internals.cpp 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/testing/Internals.cpp 2013-08-05 13:09:19 UTC (rev 153704)
@@ -393,6 +393,16 @@
return parentTreeScope ? parentTreeScope->rootNode() : 0;
}
+unsigned Internals::lastSpatialNavigationCandidateCount(ExceptionCode& ec) const
+{
+ if (!contextDocument() || !contextDocument()->page()) {
+ ec = INVALID_ACCESS_ERR;
+ return 0;
+ }
+
+ return contextDocument()->page()->lastSpatialNavigationCandidateCount();
+}
+
unsigned Internals::numberOfActiveAnimations() const
{
Frame* contextFrame = frame();
Modified: trunk/Source/WebCore/testing/Internals.h (153703 => 153704)
--- trunk/Source/WebCore/testing/Internals.h 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/testing/Internals.h 2013-08-05 13:09:19 UTC (rev 153704)
@@ -94,6 +94,9 @@
String shadowPseudoId(Element*, ExceptionCode&);
void setShadowPseudoId(Element*, const String&, ExceptionCode&);
+ // Spatial Navigation testing.
+ unsigned lastSpatialNavigationCandidateCount(ExceptionCode&) const;
+
// CSS Animation testing.
unsigned numberOfActiveAnimations() const;
bool animationsAreSuspended(Document*, ExceptionCode&) const;
Modified: trunk/Source/WebCore/testing/Internals.idl (153703 => 153704)
--- trunk/Source/WebCore/testing/Internals.idl 2013-08-05 07:58:16 UTC (rev 153703)
+++ trunk/Source/WebCore/testing/Internals.idl 2013-08-05 13:09:19 UTC (rev 153704)
@@ -54,6 +54,9 @@
[RaisesException] Node treeScopeRootNode(Node node);
[RaisesException] Node parentTreeScope(Node node);
+ // Spatial Navigation testing
+ [RaisesException] unsigned long lastSpatialNavigationCandidateCount();
+
// CSS Animation testing.
unsigned long numberOfActiveAnimations();
[RaisesException] void suspendAnimations(Document document);
