commit a5892fdff0c9 Author: decltype Date: Wed Jan 10 00:40:13 2018 +0100 Bug 712130 - Add reftest to ensure the first autofocus elements gets picked. r=bz MozReview-Commit-ID: 6UAUbqwVhTm --HG-- extra : rebase_source : 7ade8a42489bbf0be97894c18a38bd765a1da891 --- layout/reftests/bugs/712130-1-ref.html | 5 +++++ layout/reftests/bugs/712130-1.html | 5 +++++ layout/reftests/bugs/reftest.list | 1 + 3 files changed, 11 insertions(+) diff --git layout/reftests/bugs/712130-1-ref.html layout/reftests/bugs/712130-1-ref.html new file mode 100644 index 000000000000..6d07ca048250 --- /dev/null +++ layout/reftests/bugs/712130-1-ref.html @@ -0,0 +1,5 @@ + + + + + diff --git layout/reftests/bugs/712130-1.html layout/reftests/bugs/712130-1.html new file mode 100644 index 000000000000..ffbc82309366 --- /dev/null +++ layout/reftests/bugs/712130-1.html @@ -0,0 +1,5 @@ + + + + + diff --git layout/reftests/bugs/reftest.list layout/reftests/bugs/reftest.list index 764d3dbd5b3c..79e5df14fcff 100644 --- layout/reftests/bugs/reftest.list +++ layout/reftests/bugs/reftest.list @@ -1707,6 +1707,7 @@ needs-focus == 703186-1.html 703186-1-ref.html needs-focus == 703186-2.html 703186-2-ref.html needs-focus != 703186-1.html 703186-2.html == 711359-1.html 711359-1-ref.html +fuzzy-if(skiaContent,1,3) needs-focus == 712130-1.html 712130-1-ref.html == 712849-1.html 712849-1-ref.html == 713856-static.html 713856-ref.html == 713856-dynamic.html 713856-ref.html commit 5269d4159ca0 Author: decltype Date: Wed Jan 10 19:07:21 2018 +0100 Bug 712130 - Add reftest in case autofocus element is seen after PresShell init. r=bz MozReview-Commit-ID: 75IKOqiqBhL --HG-- extra : rebase_source : ec679c6f6b6c48aa88dd6c5fa6b6970e6cfa1314 --- layout/reftests/bugs/712130-2-ref.html | 3 +++ layout/reftests/bugs/712130-2.html | 4 ++++ layout/reftests/bugs/reftest.list | 1 + 3 files changed, 8 insertions(+) diff --git layout/reftests/bugs/712130-2-ref.html layout/reftests/bugs/712130-2-ref.html new file mode 100644 index 000000000000..919ac05704ae --- /dev/null +++ layout/reftests/bugs/712130-2-ref.html @@ -0,0 +1,3 @@ + + + diff --git layout/reftests/bugs/712130-2.html layout/reftests/bugs/712130-2.html new file mode 100644 index 000000000000..232bef484917 --- /dev/null +++ layout/reftests/bugs/712130-2.html @@ -0,0 +1,4 @@ + + + + diff --git layout/reftests/bugs/reftest.list layout/reftests/bugs/reftest.list index 79e5df14fcff..7978c54a5ea2 100644 --- layout/reftests/bugs/reftest.list +++ layout/reftests/bugs/reftest.list @@ -1708,6 +1708,7 @@ needs-focus == 703186-2.html 703186-2-ref.html needs-focus != 703186-1.html 703186-2.html == 711359-1.html 711359-1-ref.html fuzzy-if(skiaContent,1,3) needs-focus == 712130-1.html 712130-1-ref.html +fuzzy-if(skiaContent,1,3) needs-focus == 712130-2.html 712130-2-ref.html == 712849-1.html 712849-1-ref.html == 713856-static.html 713856-ref.html == 713856-dynamic.html 713856-ref.html commit dd080e9ebacf Author: decltype Date: Mon Jan 8 22:35:00 2018 +0100 Bug 712130 - Defer autofocus until after frame construction. r=bz The autofocus attribute on form elements forces layout in CheckIfFocusable. To avoid unpleasant FOUCs, defer autofocus processing until frames are constructed in PresShell::Initialize. Resolve the race between nsAutoFocusEvent running and page load by checking the readystate at time of event posting. Skip autofocus if the element moved to a different window in the meantime. MozReview-Commit-ID: 90jiJYJWmRg --HG-- extra : rebase_source : f94b479075df3e37ec1a658d71596c03930bab92 --- dom/base/nsDocument.cpp | 128 ++++++++++++++++++++++++++++++++++++++ dom/base/nsDocument.h | 6 ++ dom/base/nsFocusManager.cpp | 2 +- dom/base/nsIDocument.h | 3 + dom/html/nsGenericHTMLElement.cpp | 64 +------------------ layout/base/PresShell.cpp | 2 + 6 files changed, 142 insertions(+), 63 deletions(-) diff --git dom/base/nsDocument.cpp dom/base/nsDocument.cpp index 4ecc537eb54e..3ed0e93222ec 100644 --- dom/base/nsDocument.cpp +++ dom/base/nsDocument.cpp @@ -1500,6 +1500,7 @@ nsDocument::nsDocument(const char* aContentType) #ifdef DEBUG , mWillReparent(false) #endif + , mAutoFocusFired(false) { SetContentTypeInternal(nsDependentCString(aContentType)); @@ -9444,6 +9445,133 @@ nsDocument::GetTemplateContentsOwner() return mTemplateContentsOwner; } +static already_AddRefed +FindTopWindowForElement(Element* element) +{ + nsIDocument* document = element->OwnerDoc(); + if (!document) { + return nullptr; + } + + nsCOMPtr window = document->GetWindow(); + if (!window) { + return nullptr; + } + + // Trying to find the top window (equivalent to window.top). + if (nsCOMPtr top = window->GetTop()) { + window = top.forget(); + } + return window.forget(); +} + +/** + * nsAutoFocusEvent is used to dispatch a focus event for an + * nsGenericHTMLFormElement with the autofocus attribute enabled. + */ +class nsAutoFocusEvent : public Runnable +{ +public: + explicit nsAutoFocusEvent(already_AddRefed&& aElement, + already_AddRefed&& aTopWindow) + : mozilla::Runnable("nsAutoFocusEvent") + , mElement(aElement) + , mTopWindow(aTopWindow) + { + } + + NS_IMETHOD Run() override + { + nsCOMPtr currentTopWindow = + FindTopWindowForElement(mElement); + if (currentTopWindow != mTopWindow) { + // The element's top window changed from when the event was queued. + // Don't take away focus from an unrelated window. + return NS_OK; + } + + nsFocusManager* fm = nsFocusManager::GetFocusManager(); + if (!fm) { + return NS_ERROR_NULL_POINTER; + } + + nsIDocument* document = mElement->OwnerDoc(); + + // Don't steal focus from the user. + if (mTopWindow->GetFocusedNode()) { + return NS_OK; + } + + // If something is focused in the same document, ignore autofocus. + if (!fm->GetFocusedContent() || + fm->GetFocusedContent()->OwnerDoc() != document) { + mozilla::ErrorResult rv; + mElement->Focus(rv); + return rv.StealNSResult(); + } + + return NS_OK; + } +private: + nsCOMPtr mElement; + nsCOMPtr mTopWindow; +}; + +void +nsDocument::SetAutoFocusElement(Element* aAutoFocusElement) +{ + if (mAutoFocusFired) { + // Too late. + return; + } + + if (mAutoFocusElement) { + // The spec disallows multiple autofocus elements, so we consider only the + // first one to preserve the old behavior. + return; + } + + mAutoFocusElement = do_GetWeakReference(aAutoFocusElement); + TriggerAutoFocus(); +} + +void +nsDocument::TriggerAutoFocus() +{ + if (mAutoFocusFired) { + return; + } + + if (!mPresShell || !mPresShell->DidInitialize()) { + // Delay autofocus until frames are constructed so that we don't thrash + // style and layout calculations. + return; + } + + nsCOMPtr autoFocusElement = do_QueryReferent(mAutoFocusElement); + if (autoFocusElement && autoFocusElement->OwnerDoc() == this) { + mAutoFocusFired = true; + + nsCOMPtr topWindow = + FindTopWindowForElement(autoFocusElement); + if (!topWindow) { + return; + } + + // NOTE: This may be removed in the future since the spec technically + // allows autofocus after load. + nsCOMPtr topDoc = topWindow->GetExtantDoc(); + if (topDoc && topDoc->GetReadyStateEnum() == nsIDocument::READYSTATE_COMPLETE) { + return; + } + + nsCOMPtr event = + new nsAutoFocusEvent(autoFocusElement.forget(), topWindow.forget()); + nsresult rv = NS_DispatchToCurrentThread(event.forget()); + NS_ENSURE_SUCCESS_VOID(rv); + } +} + void nsDocument::SetScrollToRef(nsIURI *aDocumentURI) { diff --git dom/base/nsDocument.h dom/base/nsDocument.h index a25c4b417861..d015aae0bc8f 100644 --- dom/base/nsDocument.h +++ dom/base/nsDocument.h @@ -804,6 +804,9 @@ public: // Only BlockOnload should call this! void AsyncBlockOnload(); + virtual void SetAutoFocusElement(Element* aAutoFocusElement) override; + virtual void TriggerAutoFocus() override; + virtual void SetScrollToRef(nsIURI *aDocumentURI) override; virtual void ScrollToRef() override; virtual void ResetScrolledToRefAlready() override; @@ -1306,6 +1309,8 @@ private: RefPtr mImageMaps; + nsWeakPtr mAutoFocusElement; + nsCString mScrollToRef; uint8_t mScrolledToRefAlready : 1; uint8_t mChangeScrollPosWhenScrollingToRef : 1; @@ -1431,6 +1436,9 @@ private: public: bool mWillReparent; #endif + +private: + bool mAutoFocusFired : 1; }; class nsDocumentOnStack diff --git dom/base/nsFocusManager.cpp dom/base/nsFocusManager.cpp index dfec30f7da13..a8faa46fff3c 100644 --- dom/base/nsFocusManager.cpp +++ dom/base/nsFocusManager.cpp @@ -1603,7 +1603,7 @@ nsFocusManager::CheckIfFocusable(nsIContent* aContent, uint32_t aFlags) } // Make sure that our frames are up to date while ensuring the presshell is - // also initialized in case we come from an autofocus event. + // also initialized in case we come from a script calling focus() early. mEventHandlingNeedsFlush = false; doc->FlushPendingNotifications(FlushType::EnsurePresShellInitAndFrames); diff --git dom/base/nsIDocument.h dom/base/nsIDocument.h index 53ef3746ffc7..98d5b0d2ed15 100644 --- dom/base/nsIDocument.h +++ dom/base/nsIDocument.h @@ -2637,6 +2637,9 @@ public: virtual nsISupports* GetCurrentContentSink() = 0; + virtual void SetAutoFocusElement(Element* aAutoFocusElement) = 0; + virtual void TriggerAutoFocus() = 0; + virtual void SetScrollToRef(nsIURI *aDocumentURI) = 0; virtual void ScrollToRef() = 0; virtual void ResetScrolledToRefAlready() = 0; diff --git dom/html/nsGenericHTMLElement.cpp dom/html/nsGenericHTMLElement.cpp index 334c80ace636..775ec887ba2c 100644 --- dom/html/nsGenericHTMLElement.cpp +++ dom/html/nsGenericHTMLElement.cpp @@ -111,64 +111,6 @@ using namespace mozilla; using namespace mozilla::dom; -/** - * nsAutoFocusEvent is used to dispatch a focus event when a - * nsGenericHTMLFormElement is binded to the tree with the autofocus attribute - * enabled. - */ -class nsAutoFocusEvent : public Runnable -{ -public: - explicit nsAutoFocusEvent(nsGenericHTMLFormElement* aElement) - : mozilla::Runnable("nsAutoFocusEvent") - , mElement(aElement) - { - } - - NS_IMETHOD Run() override { - nsFocusManager* fm = nsFocusManager::GetFocusManager(); - if (!fm) { - return NS_ERROR_NULL_POINTER; - } - - nsIDocument* document = mElement->OwnerDoc(); - - nsPIDOMWindowOuter* window = document->GetWindow(); - if (!window) { - return NS_OK; - } - - // Trying to found the top window (equivalent to window.top). - if (nsCOMPtr top = window->GetTop()) { - window = top; - } - - if (window->GetFocusedNode()) { - return NS_OK; - } - - nsCOMPtr topDoc = window->GetExtantDoc(); - if (topDoc && topDoc->GetReadyStateEnum() == nsIDocument::READYSTATE_COMPLETE) { - return NS_OK; - } - - // If something is focused in the same document, ignore autofocus. - if (!fm->GetFocusedContent() || - fm->GetFocusedContent()->OwnerDoc() != document) { - mozilla::ErrorResult rv; - mElement->Focus(rv); - return rv.StealNSResult(); - } - - return NS_OK; - } -private: - // NOTE: nsGenericHTMLFormElement is saved as a nsGenericHTMLElement - // because AddRef/Release are ambiguous with nsGenericHTMLFormElement - // and Focus() is declared (and defined) in nsGenericHTMLElement class. - RefPtr mElement; -}; - NS_IMPL_ADDREF_INHERITED(nsGenericHTMLElement, nsGenericHTMLElementBase) NS_IMPL_RELEASE_INHERITED(nsGenericHTMLElement, nsGenericHTMLElementBase) @@ -1882,10 +1824,8 @@ nsGenericHTMLFormElement::BindToTree(nsIDocument* aDocument, // the document should not be already loaded and the "browser.autofocus" // preference should be 'true'. if (IsAutofocusable() && HasAttr(kNameSpaceID_None, nsGkAtoms::autofocus) && - nsContentUtils::AutoFocusEnabled()) { - nsCOMPtr event = new nsAutoFocusEvent(this); - rv = NS_DispatchToCurrentThread(event); - NS_ENSURE_SUCCESS(rv, rv); + nsContentUtils::AutoFocusEnabled() && aDocument) { + aDocument->SetAutoFocusElement(this); } // If @form is set, the element *has* to be in a document, otherwise it diff --git layout/base/PresShell.cpp layout/base/PresShell.cpp index 0c94f8516271..f851f9c7df2c 100644 --- layout/base/PresShell.cpp +++ layout/base/PresShell.cpp @@ -1817,6 +1817,8 @@ PresShell::Initialize(nscoord aWidth, nscoord aHeight) NS_ENSURE_STATE(!mHaveShutDown); } + mDocument->TriggerAutoFocus(); + NS_ASSERTION(rootFrame, "How did that happen?"); // Note: when the frame was created above it had the NS_FRAME_IS_DIRTY bit commit 0c31aff36b45 Author: decltype Date: Fri Jan 12 00:17:58 2018 +0100 Bug 712130 - Simplify focus stealing prevention for autofocus. r=bz Remove redundant check already implied by earlier condition: If the focus manager's focused content is in our document, all parent windows including the root window must have their focused node set to non-null. MozReview-Commit-ID: CryGb6mfczK --HG-- extra : rebase_source : ed5c452f68b76a656512026b5d162d8782083c2c --- dom/base/nsDocument.cpp | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git dom/base/nsDocument.cpp dom/base/nsDocument.cpp index 3ed0e93222ec..18b81abe599a 100644 --- dom/base/nsDocument.cpp +++ dom/base/nsDocument.cpp @@ -9490,27 +9490,14 @@ public: return NS_OK; } - nsFocusManager* fm = nsFocusManager::GetFocusManager(); - if (!fm) { - return NS_ERROR_NULL_POINTER; - } - - nsIDocument* document = mElement->OwnerDoc(); - // Don't steal focus from the user. if (mTopWindow->GetFocusedNode()) { return NS_OK; } - // If something is focused in the same document, ignore autofocus. - if (!fm->GetFocusedContent() || - fm->GetFocusedContent()->OwnerDoc() != document) { - mozilla::ErrorResult rv; - mElement->Focus(rv); - return rv.StealNSResult(); - } - - return NS_OK; + mozilla::ErrorResult rv; + mElement->Focus(rv); + return rv.StealNSResult(); } private: nsCOMPtr mElement;