Bug 1379280 - Only do async keyboard scrolling for a selection, not a focused element. r?smaug draft
authorRyan Hunt <rhunt@eqrion.net>
Thu, 13 Jul 2017 15:53:26 -0500
changeset 610298 09a73d3ccb6cd94a71edab9c19182068dfa25d64
parent 610297 a84f1f9bf145338dc2eccef28fe6fbaaa42369a4
child 637813 d9828cbfbe57b7c5444b2f8649114ee854ab7f44
push id68842
push userbmo:rhunt@eqrion.net
push dateTue, 18 Jul 2017 05:12:46 +0000
reviewerssmaug
bugs1379280
milestone56.0a1
Bug 1379280 - Only do async keyboard scrolling for a selection, not a focused element. r?smaug This commit changes async keyboard scrolling to be enabled only if the content to scroll is from a selection. This works around the problem of detecting whether an arbitrary element has key listeners that should prevent async key scrolling, because when they have the focus we will have disabled async key scrolling. MozReview-Commit-ID: 6HhSuGZNsMX
dom/base/nsDocument.cpp
dom/base/nsIDocument.h
dom/html/nsHTMLDocument.cpp
dom/html/nsHTMLDocument.h
gfx/layers/apz/src/FocusTarget.cpp
layout/base/PresShell.cpp
layout/base/nsIPresShell.h
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -4121,16 +4121,22 @@ nsDocument::IsNodeOfType(uint32_t aFlags
 
 Element*
 nsIDocument::GetRootElement() const
 {
   return (mCachedRootElement && mCachedRootElement->GetParentNode() == this) ?
          mCachedRootElement : GetRootElementInternal();
 }
 
+nsIContent*
+nsIDocument::GetUnfocusedKeyEventTarget()
+{
+  return GetRootElement();
+}
+
 Element*
 nsDocument::GetRootElementInternal() const
 {
   // We invoke GetRootElement() immediately before the servo traversal, so we
   // should always have a cache hit from Servo.
   MOZ_ASSERT(NS_IsMainThread());
 
   // Loop backwards because any non-elements, such as doctypes and PIs
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -934,16 +934,22 @@ public:
   mozilla::dom::DocumentType* GetDoctype() const;
 
   /**
    * Return the root element for this document.
    */
   Element* GetRootElement() const;
 
   /**
+   * Gets the event target to dispatch key events to if there is no focused
+   * content in the document.
+   */
+  virtual nsIContent* GetUnfocusedKeyEventTarget();
+
+  /**
    * Retrieve information about the viewport as a data structure.
    * This will return information in the viewport META data section
    * of the document. This can be used in lieu of ProcessViewportInfo(),
    * which places the viewport information in the document header instead
    * of returning it directly.
    *
    * @param aDisplaySize size of the on-screen display area for this
    * document, in device pixels.
--- a/dom/html/nsHTMLDocument.cpp
+++ b/dom/html/nsHTMLDocument.cpp
@@ -878,16 +878,25 @@ nsHTMLDocument::SetCompatibilityMode(nsC
   if (shell) {
     nsPresContext *pc = shell->GetPresContext();
     if (pc) {
       pc->CompatibilityModeChanged();
     }
   }
 }
 
+nsIContent*
+nsHTMLDocument::GetUnfocusedKeyEventTarget()
+{
+  if (nsGenericHTMLElement* body = GetBody()) {
+    return body;
+  }
+  return GetRootElement();
+}
+
 //
 // nsIDOMHTMLDocument interface implementation
 //
 already_AddRefed<nsIURI>
 nsHTMLDocument::GetDomainURI()
 {
   nsIPrincipal* principal = NodePrincipal();
 
--- a/dom/html/nsHTMLDocument.h
+++ b/dom/html/nsHTMLDocument.h
@@ -69,16 +69,18 @@ public:
   // nsIHTMLDocument
   virtual void SetCompatibilityMode(nsCompatibility aMode) override;
 
   virtual bool IsWriting() override
   {
     return mWriteLevel != uint32_t(0);
   }
 
+  virtual nsIContent* GetUnfocusedKeyEventTarget() override;
+
   virtual nsContentList* GetForms() override;
 
   virtual nsContentList* GetFormControls() override;
 
   // nsIDOMDocument interface
   using nsDocument::CreateElement;
   using nsDocument::CreateElementNS;
   NS_FORWARD_NSIDOMDOCUMENT(nsDocument::)
--- a/gfx/layers/apz/src/FocusTarget.cpp
+++ b/gfx/layers/apz/src/FocusTarget.cpp
@@ -89,35 +89,51 @@ FocusTarget::FocusTarget(nsIPresShell* a
   : mSequenceNumber(aFocusSequenceNumber)
   , mFocusHasKeyEventListeners(false)
 {
   MOZ_ASSERT(aRootPresShell);
   MOZ_ASSERT(NS_IsMainThread());
 
   // Key events can be retargeted to a child PresShell when there is an iframe
   nsCOMPtr<nsIPresShell> presShell = GetRetargetEventPresShell(aRootPresShell);
+  nsCOMPtr<nsIDocument> document = presShell->GetDocument();
 
-  if (!presShell) {
+  if (!presShell || !document) {
     FT_LOG("Creating nil target with seq=%" PRIu64 " (can't find retargeted presshell)\n",
            aFocusSequenceNumber);
 
     mType = FocusTarget::eNone;
     return;
   }
 
-  // Get the content that should be scrolled for this PresShell, which is
-  // the current focused element or the current DOM selection
-  nsCOMPtr<nsIContent> scrollTarget = presShell->GetContentForScrolling();
+  // Find the focused content and use it to determine whether there are key event
+  // listeners or whether key events will be targeted at a different process
+  // through a remote browser.
+  nsCOMPtr<nsIContent> focusedContent = presShell->GetFocusedContentForScrolling();
+
+  // Check if there are key event listeners that could prevent default or change
+  // the focus or selection of the page.
+  mFocusHasKeyEventListeners =
+    HasListenersForKeyEvents(focusedContent ? focusedContent.get()
+                                            : document->GetUnfocusedKeyEventTarget());
 
-  // Collect event listener information so we can track what is potentially focus
-  // changing
-  mFocusHasKeyEventListeners = HasListenersForKeyEvents(scrollTarget);
+  // Check if the focused element is content editable or if the document
+  // is in design mode.
+  if (IsEditableNode(focusedContent) ||
+      IsEditableNode(document)) {
+    FT_LOG("Creating nil target with seq=%" PRIu64 ", kl=%d (disabling for editable node)\n",
+           aFocusSequenceNumber,
+           static_cast<int>(mFocusHasKeyEventListeners));
 
-  // Check if the scroll target is a remote browser
-  if (TabParent* browserParent = TabParent::GetFrom(scrollTarget)) {
+    mType = FocusTarget::eNone;
+    return;
+  }
+
+  // Check if the focused element is a remote browser
+  if (TabParent* browserParent = TabParent::GetFrom(focusedContent)) {
     RenderFrameParent* rfp = browserParent->GetRenderFrame();
 
     // The globally focused element for scrolling is in a remote layer tree
     if (rfp) {
       FT_LOG("Creating reflayer target with seq=%" PRIu64 ", kl=%d, lt=%" PRIu64 "\n",
              aFocusSequenceNumber,
              mFocusHasKeyEventListeners,
              rfp->GetLayersId());
@@ -130,34 +146,39 @@ FocusTarget::FocusTarget(nsIPresShell* a
     FT_LOG("Creating nil target with seq=%" PRIu64 ", kl=%d (remote browser missing layers id)\n",
            aFocusSequenceNumber,
            mFocusHasKeyEventListeners);
 
     mType = FocusTarget::eNone;
     return;
   }
 
-  // If the focus isn't on a remote browser then check for scrollable targets
-  if (IsEditableNode(scrollTarget) ||
-      IsEditableNode(presShell->GetDocument())) {
-    FT_LOG("Creating nil target with seq=%" PRIu64 ", kl=%d (disabling for editable node)\n",
-           aFocusSequenceNumber,
-           mFocusHasKeyEventListeners);
+  // The content to scroll is either the focused element or the focus node of
+  // the selection. It's difficult to determine if an element is an interactive
+  // element requiring async keyboard scrolling to be disabled. So we only
+  // allow async key scrolling based on the selection, which doesn't have
+  // this problem and is more common.
+  if (focusedContent) {
+    FT_LOG("Creating nil target with seq=%" PRIu64 ", kl=%d (disabling for focusing an element)\n",
+           mFocusHasKeyEventListeners,
+           aFocusSequenceNumber);
 
     mType = FocusTarget::eNone;
     return;
   }
 
+  nsCOMPtr<nsIContent> selectedContent = presShell->GetSelectedContentForScrolling();
+
   // Gather the scrollable frames that would be scrolled in each direction
   // for this scroll target
   nsIScrollableFrame* horizontal =
-    presShell->GetScrollableFrameToScrollForContent(scrollTarget.get(),
+    presShell->GetScrollableFrameToScrollForContent(selectedContent.get(),
                                                     nsIPresShell::eHorizontal);
   nsIScrollableFrame* vertical =
-    presShell->GetScrollableFrameToScrollForContent(scrollTarget.get(),
+    presShell->GetScrollableFrameToScrollForContent(selectedContent.get(),
                                                     nsIPresShell::eVertical);
 
   // We might have the globally focused element for scrolling. Gather a ViewID for
   // the horizontal and vertical scroll targets of this element.
   mType = FocusTarget::eScrollLayer;
   mData.mScrollTargets.mHorizontal =
     nsLayoutUtils::FindIDForScrollableFrame(horizontal);
   mData.mScrollTargets.mVertical =
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -2835,25 +2835,41 @@ PresShell::FrameNeedsToContinueReflow(ns
                "Frame passed in not in reflow?");
 
   mFramesToDirty.PutEntry(aFrame);
 }
 
 already_AddRefed<nsIContent>
 nsIPresShell::GetContentForScrolling() const
 {
+  if (nsCOMPtr<nsIContent> focused = GetFocusedContentForScrolling()) {
+    return focused.forget();
+  }
+  return GetSelectedContentForScrolling();
+}
+
+already_AddRefed<nsIContent>
+nsIPresShell::GetFocusedContentForScrolling() const
+{
   nsCOMPtr<nsIContent> focusedContent;
   nsIFocusManager* fm = nsFocusManager::GetFocusManager();
   if (fm && mDocument) {
     nsCOMPtr<nsIDOMElement> focusedElement;
     fm->GetFocusedElementForWindow(mDocument->GetWindow(), false, nullptr,
                                    getter_AddRefs(focusedElement));
     focusedContent = do_QueryInterface(focusedElement);
   }
-  if (!focusedContent && mSelection) {
+  return focusedContent.forget();
+}
+
+already_AddRefed<nsIContent>
+nsIPresShell::GetSelectedContentForScrolling() const
+{
+  nsCOMPtr<nsIContent> focusedContent;
+  if (mSelection) {
     nsISelection* domSelection =
       mSelection->GetSelection(SelectionType::eNormal);
     if (domSelection) {
       nsCOMPtr<nsIDOMNode> focusedNode;
       domSelection->GetFocusNode(getter_AddRefs(focusedNode));
       focusedContent = do_QueryInterface(focusedNode);
     }
   }
@@ -7759,27 +7775,17 @@ PresShell::HandleEvent(nsIFrame* aFrame,
         nsFocusManager::GetFocusedDescendant(window, false,
                                              getter_AddRefs(focusedWindow));
 
       // otherwise, if there is no focused content or the focused content has
       // no frame, just use the root content. This ensures that key events
       // still get sent to the window properly if nothing is focused or if a
       // frame goes away while it is focused.
       if (!eventTarget || !eventTarget->GetPrimaryFrame()) {
-        nsCOMPtr<nsIDOMHTMLDocument> htmlDoc = do_QueryInterface(mDocument);
-        if (htmlDoc) {
-          nsCOMPtr<nsIDOMHTMLElement> body;
-          htmlDoc->GetBody(getter_AddRefs(body));
-          eventTarget = do_QueryInterface(body);
-          if (!eventTarget) {
-            eventTarget = mDocument->GetRootElement();
-          }
-        } else {
-          eventTarget = mDocument->GetRootElement();
-        }
+        eventTarget = mDocument->GetUnfocusedKeyEventTarget();
       }
 
       if (aEvent->mMessage == eKeyDown) {
         NS_IF_RELEASE(gKeyDownTarget);
         NS_IF_ADDREF(gKeyDownTarget = eventTarget);
       }
       else if ((aEvent->mMessage == eKeyPress ||
                 aEvent->mMessage == eKeyUp) &&
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -430,16 +430,27 @@ public:
 
   /**
    * Get the current focused content or DOM selection that should be the
    * target for scrolling.
    */
   already_AddRefed<nsIContent> GetContentForScrolling() const;
 
   /**
+   * Get the focused content that should be the target for scrolling.
+   */
+  already_AddRefed<nsIContent> GetFocusedContentForScrolling() const;
+
+  /**
+   * Get the DOM selection that should be the target for scrolling, if there
+   * is no focused content.
+   */
+  already_AddRefed<nsIContent> GetSelectedContentForScrolling() const;
+
+  /**
    * Gets nearest scrollable frame from the specified content node. The frame
    * is scrollable with overflow:scroll or overflow:auto in some direction when
    * aDirection is eEither.  Otherwise, this returns a nearest frame that is
    * scrollable in the specified direction.
    */
   enum ScrollDirection { eHorizontal, eVertical, eEither };
   nsIScrollableFrame* GetScrollableFrameToScrollForContent(
                          nsIContent* aContent,