Bug 1424633: Decide which frame to focus using the flattened tree. r=smaug
☠☠ backed out by e35b0ca176e1 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 14 Dec 2017 00:07:50 +0100
changeset 396559 b83870070900d6d4c487b830e9b1342de474142d
parent 396558 eb72a70b524f7434a307df720019efa7442a6554
child 396560 d35b7eba50744683e17c2792f8f4129b0b0cb4b0
push id57045
push userecoal95@gmail.com
push dateFri, 15 Dec 2017 18:26:20 +0000
treeherderautoland@b83870070900 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1424633
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1424633: Decide which frame to focus using the flattened tree. r=smaug MozReview-Commit-ID: vPbPvu0vrf
dom/events/EventStateManager.cpp
dom/events/test/mochitest.ini
dom/events/test/test_focus_abspos.html
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -3159,74 +3159,74 @@ EventStateManager::PostHandleEvent(nsPre
             }
           }
         }
 
         if (!suppressBlur) {
           suppressBlur = nsContentUtils::IsUserFocusIgnored(activeContent);
         }
 
-        nsIFrame* currFrame = mCurrentTarget;
-
         // When a root content which isn't editable but has an editable HTML
         // <body> element is clicked, we should redirect the focus to the
         // the <body> element.  E.g., when an user click bottom of the editor
         // where is outside of the <body> element, the <body> should be focused
         // and the user can edit immediately after that.
         //
         // NOTE: The newFocus isn't editable that also means it's not in
         // designMode.  In designMode, all contents are not focusable.
         if (newFocus && !newFocus->IsEditable()) {
           nsIDocument *doc = newFocus->GetComposedDoc();
           if (doc && newFocus == doc->GetRootElement()) {
             nsIContent *bodyContent =
               nsLayoutUtils::GetEditableRootContentByContentEditable(doc);
-            if (bodyContent) {
-              nsIFrame* bodyFrame = bodyContent->GetPrimaryFrame();
-              if (bodyFrame) {
-                currFrame = bodyFrame;
-                newFocus = bodyContent;
-              }
+            if (bodyContent && bodyContent->GetPrimaryFrame()) {
+              newFocus = bodyContent;
             }
           }
         }
 
         // When the mouse is pressed, the default action is to focus the
         // target. Look for the nearest enclosing focusable frame.
-        while (currFrame) {
-          // If the mousedown happened inside a popup, don't
-          // try to set focus on one of its containing elements
-          const nsStyleDisplay* display = currFrame->StyleDisplay();
-          if (display->mDisplay == StyleDisplay::MozPopup) {
+        //
+        // TODO: Probably this should be moved to Element::PostHandleEvent.
+        for (; newFocus; newFocus = newFocus->GetFlattenedTreeParent()) {
+          if (!newFocus->IsElement()) {
+            continue;
+          }
+
+          nsIFrame* frame = newFocus->GetPrimaryFrame();
+          if (!frame) {
+            continue;
+          }
+
+          // If the mousedown happened inside a popup, don't try to set focus on
+          // one of its containing elements
+          if (frame->StyleDisplay()->mDisplay == StyleDisplay::MozPopup) {
             newFocus = nullptr;
             break;
           }
 
           int32_t tabIndexUnused;
-          if (currFrame->IsFocusable(&tabIndexUnused, true)) {
-            newFocus = currFrame->GetContent();
-            nsCOMPtr<nsIDOMElement> domElement(do_QueryInterface(newFocus));
-            if (domElement)
-              break;
+          if (frame->IsFocusable(&tabIndexUnused, true)) {
+            break;
           }
-          currFrame = currFrame->GetParent();
         }
 
         nsIFocusManager* fm = nsFocusManager::GetFocusManager();
         if (fm) {
           // if something was found to focus, focus it. Otherwise, if the
           // element that was clicked doesn't have -moz-user-focus: ignore,
           // clear the existing focus. For -moz-user-focus: ignore, the focus
           // is just left as is.
           // Another effect of mouse clicking, handled in nsSelection, is that
           // it should update the caret position to where the mouse was
           // clicked. Because the focus is cleared when clicking on a
           // non-focusable node, the next press of the tab key will cause
           // focus to be shifted from the caret position instead of the root.
-          if (newFocus && currFrame) {
+          if (newFocus) {
             // use the mouse flag and the noscroll flag so that the content
             // doesn't unexpectedly scroll when clicking an element that is
             // only half visible
             uint32_t flags = nsIFocusManager::FLAG_BYMOUSE |
                              nsIFocusManager::FLAG_NOSCROLL;
             // If this was a touch-generated event, pass that information:
             if (mouseEvent->inputSource == nsIDOMMouseEvent::MOZ_SOURCE_TOUCH) {
               flags |= nsIFocusManager::FLAG_BYTOUCH;
--- a/dom/events/test/mochitest.ini
+++ b/dom/events/test/mochitest.ini
@@ -165,16 +165,17 @@ skip-if = toolkit == 'android' #CRASH_DU
 [test_dragstart.html]
 [test_error_events.html]
 skip-if = toolkit == 'android' #TIMED_OUT
 [test_eventctors.html]
 skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM
 [test_eventhandler_scoping.html]
 [test_eventTimeStamp.html]
 [test_focus_disabled.html]
+[test_focus_abspos.html]
 [test_legacy_event.html]
 [test_messageEvent.html]
 [test_messageEvent_init.html]
 [test_moz_mouse_pixel_scroll_event.html]
 [test_offsetxy.html]
 [test_onerror_handler_args.html]
 [test_passive_listeners.html]
 [test_paste_image.html]
new file mode 100644
--- /dev/null
+++ b/dom/events/test/test_focus_abspos.html
@@ -0,0 +1,30 @@
+<!doctype html>
+<title>Test for bug 1424633: clicking on an oof descendant focus its focusable ancestor</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<style>
+  #focusable {
+    width: 100px;
+    height: 100px;
+    background-color: blue;
+  }
+  #oof {
+    background-color: green;
+    position: absolute;
+    top: 25px;
+  }
+</style>
+<div tabindex="0" id="focusable">
+  <span id="oof">Absolute</span>
+</div>
+<script>
+async_test(function(t) {
+  document.body.offsetTop;
+  setTimeout(t.step_func_done(function() {
+    let span = document.querySelector("#oof");
+    synthesizeMouseAtCenter(span, {type: "mousedown"});
+    assert_equals(document.activeElement, document.querySelector("#focusable"));
+  }), 0);
+}, "Clicking on an abspos descendant focus its focusable ancestor");
+</script>