Bug 1328043, scrolling into view on focus should happen even when event shouldn't fire, fixes tabbing into blank iframes, r=smaug
authorNeil Deakin <neil@mozilla.com>
Mon, 09 Jan 2017 11:03:52 -0500
changeset 356576 15748795cb4b4450cbb198b030af29b7f273d46e
parent 356575 28ca0dfe8d723df5c41a5d79114a5e42609e3a3c
child 356577 afa19305d88a6506b953300be9757a8f9c4931ce
push id10621
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 16:02:43 +0000
treeherdermozilla-aurora@dca7b42e6c67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1328043
milestone53.0a1
Bug 1328043, scrolling into view on focus should happen even when event shouldn't fire, fixes tabbing into blank iframes, r=smaug
dom/base/nsFocusManager.cpp
dom/tests/mochitest/general/mochitest.ini
dom/tests/mochitest/general/test_focus_scrollchildframe.html
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -1895,24 +1895,25 @@ nsFocusManager::Focus(nsPIDOMWindowOuter
       mFocusedWindow == aWindow && mFocusedContent == nullptr) {
     mFocusedContent = aContent;
 
     nsIContent* focusedNode = aWindow->GetFocusedNode();
     bool isRefocus = focusedNode && focusedNode->IsEqualNode(aContent);
 
     aWindow->SetFocusedNode(aContent, focusMethod);
 
+    // if the focused element changed, scroll it into view
+    if (aContent && aFocusChanged) {
+      ScrollIntoView(presShell, aContent, aFlags);
+    }
+
     bool sendFocusEvent =
       aContent && aContent->IsInComposedDoc() && !IsNonFocusableRoot(aContent);
     nsPresContext* presContext = presShell->GetPresContext();
     if (sendFocusEvent) {
-      // if the focused element changed, scroll it into view
-      if (aFocusChanged)
-        ScrollIntoView(presShell, aContent, aFlags);
-
       NotifyFocusStateChange(aContent, aWindow->ShouldShowFocusRing(), true);
 
       // if this is an object/plug-in/remote browser, focus its widget.  Note that we might
       // no longer be in the same document, due to the events we fired above when
       // aIsNewDocument.
       if (presShell->GetDocument() == aContent->GetComposedDoc()) {
         if (aAdjustWidgets && objectFrameWidget && !sTestMode)
           objectFrameWidget->SetFocus(false);
--- a/dom/tests/mochitest/general/mochitest.ini
+++ b/dom/tests/mochitest/general/mochitest.ini
@@ -79,16 +79,17 @@ subsuite = clipboard
 subsuite = clipboard
 [test_consoleAPI.html]
 [test_contentViewer_overrideDPPX.html]
 [test_DOMMatrix.html]
 [test_domWindowUtils.html]
 [test_domWindowUtils_scrollbarSize.html]
 [test_domWindowUtils_scrollXY.html]
 [test_donottrack.html]
+[test_focus_scrollchildframe.html]
 [test_focus_legend_noparent.html]
 [test_focusrings.xul]
 skip-if = toolkit == 'android' #TIMED_OUT
 [test_for_of.html]
 [test_framedhistoryframes.html]
 [test_frameElementWrapping.html]
 [test_img_mutations.html]
 [test_interfaces.html]
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/general/test_focus_scrollchildframe.html
@@ -0,0 +1,24 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Tests for for-of loops</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css">
+</head>
+<body onload="doTest()">
+<p id="display"></p>
+<div id="content" style="display: none"></div>
+<script>
+function doTest() {
+  document.getElementById("button").focus();
+  is(window.scrollY, 0, "Scrolled position initially 0");
+  synthesizeKey("VK_TAB", { }),
+  ok(window.scrollY > 200, "Scrolled child frame into view");
+  SimpleTest.finish();
+}
+SimpleTest.waitForExplicitFinish();
+</script>
+<button id="button">B</button><br><iframe style="margin-top:10000px;height:400px"></iframe>
+</body>
+</html>