From c21eecd1b3ac66b2482e6d8ad6fa9174bac56655 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sat, 13 Jun 2020 08:17:56 -0400 Subject: [PATCH 01/13] Bug 1217436 - Make nsIdentifierMapEntry::mIdContentList an AutoTArray to save an allocation Tag #1375 --- dom/base/nsIdentifierMapEntry.h | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/dom/base/nsIdentifierMapEntry.h b/dom/base/nsIdentifierMapEntry.h index fce506cef3..ea60d33221 100644 --- a/dom/base/nsIdentifierMapEntry.h +++ b/dom/base/nsIdentifierMapEntry.h @@ -54,13 +54,18 @@ public: nsStringHashKey(aKey), mNameContentList(nullptr) { } - nsIdentifierMapEntry(const nsIdentifierMapEntry& aOther) : - nsStringHashKey(&aOther.GetKey()) + nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther) : + nsStringHashKey(&aOther.GetKey()), + mIdContentList(mozilla::Move(aOther.mIdContentList)), + mNameContentList(aOther.mNameContentList.forget()), + mChangeCallbacks(aOther.mChangeCallbacks.forget()), + mImageElement(aOther.mImageElement.forget()) { - NS_ERROR("Should never be called"); } ~nsIdentifierMapEntry(); + enum { ALLOW_MEMMOVE = false }; + void AddNameElement(nsINode* aDocument, Element* aElement); void RemoveNameElement(Element* aElement); bool IsEmpty(); @@ -156,12 +161,15 @@ public: size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const; private: + nsIdentifierMapEntry(const nsIdentifierMapEntry& aOther) = delete; + nsIdentifierMapEntry& operator=(const nsIdentifierMapEntry& aOther) = delete; + void FireChangeCallbacks(Element* aOldElement, Element* aNewElement, bool aImageOnly = false); // empty if there are no elements with this ID. // The elements are stored as weak pointers. - nsTArray mIdContentList; + AutoTArray mIdContentList; RefPtr mNameContentList; nsAutoPtr > mChangeCallbacks; RefPtr mImageElement; From 15f6d2efdafbdf946e9d4da861fb38e25f81be9a Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sat, 13 Jun 2020 08:18:46 -0400 Subject: [PATCH 02/13] Bug 1355787 - nsIdentifierMapEntry should let one to use either strings or atoms as keys to avoid slow string assignments when possible. Tag #1375 --- dom/base/ShadowRoot.cpp | 6 +-- dom/base/nsDocument.cpp | 18 ++++----- dom/base/nsIdentifierMapEntry.h | 72 ++++++++++++++++++++++++++++++--- dom/html/nsHTMLDocument.cpp | 2 +- 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp index 2383c951a6..d6c103e8ef 100644 --- a/dom/base/ShadowRoot.cpp +++ b/dom/base/ShadowRoot.cpp @@ -319,8 +319,7 @@ ShadowRoot::GetElementsByTagNameNS(const nsAString& aNamespaceURI, void ShadowRoot::AddToIdTable(Element* aElement, nsIAtom* aId) { - nsIdentifierMapEntry *entry = - mIdentifierMap.PutEntry(nsDependentAtomString(aId)); + nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aId); if (entry) { entry->AddIdElement(aElement); } @@ -329,8 +328,7 @@ ShadowRoot::AddToIdTable(Element* aElement, nsIAtom* aId) void ShadowRoot::RemoveFromIdTable(Element* aElement, nsIAtom* aId) { - nsIdentifierMapEntry *entry = - mIdentifierMap.GetEntry(nsDependentAtomString(aId)); + nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aId); if (entry) { entry->RemoveIdElement(aElement); if (entry->IsEmpty()) { diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 7b280a188d..203c44cbf4 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -537,7 +537,7 @@ nsIdentifierMapEntry::HasIdElementExposedAsHTMLDocumentProperty() size_t nsIdentifierMapEntry::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const { - return nsStringHashKey::SizeOfExcludingThis(aMallocSizeOf); + return mKey.mString.SizeOfExcludingThisIfUnshared(aMallocSizeOf); } // Helper structs for the content->subdoc map @@ -2704,8 +2704,7 @@ nsDocument::AddToNameTable(Element *aElement, nsIAtom* aName) "Only put elements that need to be exposed as document['name'] in " "the named table."); - nsIdentifierMapEntry *entry = - mIdentifierMap.PutEntry(nsDependentAtomString(aName)); + nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aName); // Null for out-of-memory if (entry) { @@ -2724,8 +2723,7 @@ nsDocument::RemoveFromNameTable(Element *aElement, nsIAtom* aName) if (mIdentifierMap.Count() == 0) return; - nsIdentifierMapEntry *entry = - mIdentifierMap.GetEntry(nsDependentAtomString(aName)); + nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aName); if (!entry) // Could be false if the element was anonymous, hence never added return; @@ -2739,8 +2737,7 @@ nsDocument::RemoveFromNameTable(Element *aElement, nsIAtom* aName) void nsDocument::AddToIdTable(Element *aElement, nsIAtom* aId) { - nsIdentifierMapEntry *entry = - mIdentifierMap.PutEntry(nsDependentAtomString(aId)); + nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aId); if (entry) { /* True except on OOM */ if (nsGenericHTMLElement::ShouldExposeIdAsHTMLDocumentProperty(aElement) && @@ -2762,8 +2759,7 @@ nsDocument::RemoveFromIdTable(Element *aElement, nsIAtom* aId) return; } - nsIdentifierMapEntry *entry = - mIdentifierMap.GetEntry(nsDependentAtomString(aId)); + nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aId); if (!entry) // Can be null for XML elements with changing ids. return; @@ -4783,7 +4779,7 @@ nsDocument::AddIDTargetObserver(nsIAtom* aID, IDTargetObserver aObserver, if (!CheckGetElementByIdArg(id)) return nullptr; - nsIdentifierMapEntry *entry = mIdentifierMap.PutEntry(id); + nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aID); NS_ENSURE_TRUE(entry, nullptr); entry->AddContentChangeCallback(aObserver, aData, aForImage); @@ -4799,7 +4795,7 @@ nsDocument::RemoveIDTargetObserver(nsIAtom* aID, IDTargetObserver aObserver, if (!CheckGetElementByIdArg(id)) return; - nsIdentifierMapEntry *entry = mIdentifierMap.GetEntry(id); + nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aID); if (!entry) { return; } diff --git a/dom/base/nsIdentifierMapEntry.h b/dom/base/nsIdentifierMapEntry.h index ea60d33221..41b8b4a836 100644 --- a/dom/base/nsIdentifierMapEntry.h +++ b/dom/base/nsIdentifierMapEntry.h @@ -40,22 +40,45 @@ class nsIContent; * Perhaps the document.all results should have their own hashtable * in nsHTMLDocument. */ -class nsIdentifierMapEntry : public nsStringHashKey +class nsIdentifierMapEntry : public PLDHashEntryHdr { public: + struct AtomOrString + { + MOZ_IMPLICIT AtomOrString(nsIAtom* aAtom) : mAtom(aAtom) {} + MOZ_IMPLICIT AtomOrString(const nsAString& aString) : mString(aString) {} + AtomOrString(const AtomOrString& aOther) + : mAtom(aOther.mAtom) + , mString(aOther.mString) + { + } + + AtomOrString(AtomOrString&& aOther) + : mAtom(aOther.mAtom.forget()) + , mString(aOther.mString) + { + } + + nsCOMPtr mAtom; + const nsString mString; + }; + + typedef const AtomOrString& KeyType; + typedef const AtomOrString* KeyTypePointer; + typedef mozilla::dom::Element Element; typedef mozilla::net::ReferrerPolicy ReferrerPolicy; - explicit nsIdentifierMapEntry(const nsAString& aKey) : - nsStringHashKey(&aKey), mNameContentList(nullptr) + explicit nsIdentifierMapEntry(const AtomOrString& aKey) + : mKey(aKey) { } - explicit nsIdentifierMapEntry(const nsAString* aKey) : - nsStringHashKey(aKey), mNameContentList(nullptr) + explicit nsIdentifierMapEntry(const AtomOrString* aKey) + : mKey(aKey ? *aKey : nullptr) { } nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther) : - nsStringHashKey(&aOther.GetKey()), + mKey(mozilla::Move(aOther.GetKey())), mIdContentList(mozilla::Move(aOther.mIdContentList)), mNameContentList(aOther.mNameContentList.forget()), mChangeCallbacks(aOther.mChangeCallbacks.forget()), @@ -64,6 +87,42 @@ public: } ~nsIdentifierMapEntry(); + KeyType GetKey() const { return mKey; } + + nsString GetKeyAsString() const + { + if (mKey.mAtom) { + return nsAtomString(mKey.mAtom); + } + + return mKey.mString; + } + + bool KeyEquals(const KeyTypePointer aOtherKey) const + { + if (mKey.mAtom) { + if (aOtherKey->mAtom) { + return mKey.mAtom == aOtherKey->mAtom; + } + + return mKey.mAtom->Equals(aOtherKey->mString); + } + + if (aOtherKey->mAtom) { + return aOtherKey->mAtom->Equals(mKey.mString); + } + + return mKey.mString.Equals(aOtherKey->mString); + } + + static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; } + + static PLDHashNumber HashKey(const KeyTypePointer aKey) + { + return aKey->mAtom ? + aKey->mAtom->hash() : mozilla::HashString(aKey->mString); + } + enum { ALLOW_MEMMOVE = false }; void AddNameElement(nsINode* aDocument, Element* aElement); @@ -167,6 +226,7 @@ private: void FireChangeCallbacks(Element* aOldElement, Element* aNewElement, bool aImageOnly = false); + AtomOrString mKey; // empty if there are no elements with this ID. // The elements are stored as weak pointers. AutoTArray mIdContentList; diff --git a/dom/html/nsHTMLDocument.cpp b/dom/html/nsHTMLDocument.cpp index bd9de39f03..f3cb096b9a 100644 --- a/dom/html/nsHTMLDocument.cpp +++ b/dom/html/nsHTMLDocument.cpp @@ -2094,7 +2094,7 @@ nsHTMLDocument::GetSupportedNames(nsTArray& aNames) nsIdentifierMapEntry* entry = iter.Get(); if (entry->HasNameElement() || entry->HasIdElementExposedAsHTMLDocumentProperty()) { - aNames.AppendElement(entry->GetKey()); + aNames.AppendElement(entry->GetKeyAsString()); } } } From db72493615b7d5b3435837b6ccc1a0de681e5ff0 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sat, 13 Jun 2020 08:21:48 -0400 Subject: [PATCH 03/13] Bug 1426494 - Share more code between nsIDocument and ShadowRoot Tag #1375 --- dom/base/ChromeNodeList.cpp | 1 + dom/base/ChromeNodeList.h | 1 + dom/base/DocumentOrShadowRoot.cpp | 104 ++++++++++++++++++++ dom/base/DocumentOrShadowRoot.h | 151 ++++++++++++++++++++++++++++++ dom/base/Element.h | 1 + dom/base/ShadowRoot.cpp | 42 +-------- dom/base/ShadowRoot.h | 27 ++---- dom/base/StyleScope.cpp | 27 ------ dom/base/StyleScope.h | 81 ---------------- dom/base/StyleSheetList.cpp | 12 +-- dom/base/StyleSheetList.h | 14 +-- dom/base/moz.build | 4 +- dom/base/nsContentUtils.cpp | 8 ++ dom/base/nsContentUtils.h | 2 + dom/base/nsDocument.cpp | 82 +++++----------- dom/base/nsDocument.h | 19 ++-- dom/base/nsIDocument.h | 42 ++------- dom/base/nsIdentifierMapEntry.h | 54 +++++------ dom/base/nsNameSpaceManager.h | 2 +- dom/html/nsHTMLDocument.h | 5 +- dom/xul/XULDocument.cpp | 19 +--- dom/xul/XULDocument.h | 5 +- editor/libeditor/TextEditor.cpp | 1 + 23 files changed, 369 insertions(+), 335 deletions(-) create mode 100644 dom/base/DocumentOrShadowRoot.cpp create mode 100644 dom/base/DocumentOrShadowRoot.h delete mode 100644 dom/base/StyleScope.cpp delete mode 100644 dom/base/StyleScope.h diff --git a/dom/base/ChromeNodeList.cpp b/dom/base/ChromeNodeList.cpp index ffa1019718..2e9f058231 100644 --- a/dom/base/ChromeNodeList.cpp +++ b/dom/base/ChromeNodeList.cpp @@ -6,6 +6,7 @@ #include "mozilla/dom/ChromeNodeList.h" #include "mozilla/dom/ChromeNodeListBinding.h" +#include "nsPIDOMWindow.h" using namespace mozilla; using namespace mozilla::dom; diff --git a/dom/base/ChromeNodeList.h b/dom/base/ChromeNodeList.h index 9908808ac3..8dbd42be30 100644 --- a/dom/base/ChromeNodeList.h +++ b/dom/base/ChromeNodeList.h @@ -6,6 +6,7 @@ #include "nsCOMArray.h" #include "nsContentList.h" +#include "nsIDocument.h" namespace mozilla { class ErrorResult; diff --git a/dom/base/DocumentOrShadowRoot.cpp b/dom/base/DocumentOrShadowRoot.cpp new file mode 100644 index 0000000000..8376ab97e4 --- /dev/null +++ b/dom/base/DocumentOrShadowRoot.cpp @@ -0,0 +1,104 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "DocumentOrShadowRoot.h" +#include "mozilla/dom/StyleSheetList.h" +#include "ShadowRoot.h" +#include "XULDocument.h" + +class nsINode; +class nsIDocument; +class ShadowRoot; + +namespace mozilla { +namespace dom { + +DocumentOrShadowRoot::DocumentOrShadowRoot(mozilla::dom::ShadowRoot* aShadowRoot) + : mAsNode(aShadowRoot) + , mKind(Kind::ShadowRoot) +{ + MOZ_ASSERT(mAsNode); +} + +DocumentOrShadowRoot::DocumentOrShadowRoot(nsIDocument* aDoc) + : mAsNode(aDoc) + , mKind(Kind::Document) +{ + MOZ_ASSERT(mAsNode); +} + +StyleSheetList& +DocumentOrShadowRoot::EnsureDOMStyleSheets() +{ + if (!mDOMStyleSheets) { + mDOMStyleSheets = new StyleSheetList(*this); + } + return *mDOMStyleSheets; +} + +Element* +DocumentOrShadowRoot::GetElementById(const nsAString& aElementId) +{ + if (MOZ_UNLIKELY(aElementId.IsEmpty())) { + nsContentUtils::ReportEmptyGetElementByIdArg(AsNode().OwnerDoc()); + return nullptr; + } + + if (nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aElementId)) { + if (Element* el = entry->GetIdElement()) { + return el; + } + } + + if (MOZ_UNLIKELY(mKind == Kind::Document && + static_cast(AsNode()).IsXULDocument())) { + return static_cast(AsNode()).GetRefById(aElementId); + } + + return nullptr; +} + +already_AddRefed +DocumentOrShadowRoot::GetElementsByTagNameNS(const nsAString& aNamespaceURI, + const nsAString& aLocalName) +{ + ErrorResult rv; + RefPtr list = + GetElementsByTagNameNS(aNamespaceURI, aLocalName, rv); + if (rv.Failed()) { + return nullptr; + } + return list.forget(); +} + +already_AddRefed +DocumentOrShadowRoot::GetElementsByTagNameNS(const nsAString& aNamespaceURI, + const nsAString& aLocalName, + mozilla::ErrorResult& aResult) +{ + int32_t nameSpaceId = kNameSpaceID_Wildcard; + + if (!aNamespaceURI.EqualsLiteral("*")) { + aResult = + nsContentUtils::NameSpaceManager()->RegisterNameSpace(aNamespaceURI, + nameSpaceId); + if (aResult.Failed()) { + return nullptr; + } + } + + NS_ASSERTION(nameSpaceId != kNameSpaceID_Unknown, "Unexpected namespace ID!"); + return NS_GetContentList(&AsNode(), nameSpaceId, aLocalName); +} + +already_AddRefed +DocumentOrShadowRoot::GetElementsByClassName(const nsAString& aClasses) +{ + return nsContentUtils::GetElementsByClassName(&AsNode(), aClasses); +} + +} +} diff --git a/dom/base/DocumentOrShadowRoot.h b/dom/base/DocumentOrShadowRoot.h new file mode 100644 index 0000000000..59be0c2a9b --- /dev/null +++ b/dom/base/DocumentOrShadowRoot.h @@ -0,0 +1,151 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_dom_DocumentOrShadowRoot_h__ +#define mozilla_dom_DocumentOrShadowRoot_h__ + +#include "nsTArray.h" +#include "nsIdentifierMapEntry.h" +#include "nsContentListDeclarations.h" +#include "nsNameSpaceManager.h" +#include "mozilla/dom/NameSpaceConstants.h" + +class nsContentList; +class nsINode; + +namespace mozilla { +class StyleSheet; + +namespace dom { + +class StyleSheetList; +class ShadowRoot; + +/** + * A class meant to be shared by ShadowRoot and Document, that holds a list of + * stylesheets. + * + * TODO(emilio, bug 1418159): In the future this should hold most of the + * relevant style state, this should allow us to fix bug 548397. + */ +class DocumentOrShadowRoot +{ + enum class Kind { + Document, + ShadowRoot, + }; + +public: + explicit DocumentOrShadowRoot(nsIDocument*); + explicit DocumentOrShadowRoot(mozilla::dom::ShadowRoot*); + + nsINode& AsNode() + { + return *mAsNode; + } + + const nsINode& AsNode() const + { + return *mAsNode; + } + + StyleSheet* SheetAt(size_t aIndex) const + { + return mStyleSheets.SafeElementAt(aIndex); + } + + size_t SheetCount() const + { + return mStyleSheets.Length(); + } + + int32_t IndexOfSheet(const StyleSheet& aSheet) const + { + return mStyleSheets.IndexOf(&aSheet); + } + + void InsertSheetAt(size_t aIndex, StyleSheet& aSheet) + { + mStyleSheets.InsertElementAt(aIndex, &aSheet); + } + + void RemoveSheet(StyleSheet& aSheet) + { + mStyleSheets.RemoveElement(&aSheet); + } + + void AppendStyleSheet(StyleSheet& aSheet) + { + mStyleSheets.AppendElement(&aSheet); + } + + StyleSheetList& EnsureDOMStyleSheets(); + + Element* GetElementById(const nsAString& aElementId); + + /** + * This method returns _all_ the elements in this scope which have id + * aElementId, if there are any. Otherwise it returns null. + * + * This is useful for stuff like QuerySelector optimization and such. + */ + inline const nsTArray* + GetAllElementsForId(const nsAString& aElementId) const; + + already_AddRefed + GetElementsByTagName(const nsAString& aTagName) + { + return NS_GetContentList(&AsNode(), kNameSpaceID_Unknown, aTagName); + } + + already_AddRefed + GetElementsByTagNameNS(const nsAString& aNamespaceURI, + const nsAString& aLocalName); + + already_AddRefed + GetElementsByTagNameNS(const nsAString& aNamespaceURI, + const nsAString& aLocalName, + mozilla::ErrorResult&); + + already_AddRefed + GetElementsByClassName(const nsAString& aClasses); + + ~DocumentOrShadowRoot() = default; + +protected: + nsTArray> mStyleSheets; + RefPtr mDOMStyleSheets; + + /* + * mIdentifierMap works as follows for IDs: + * 1) Attribute changes affect the table immediately (removing and adding + * entries as needed). + * 2) Removals from the DOM affect the table immediately + * 3) Additions to the DOM always update existing entries for names, and add + * new ones for IDs. + */ + nsTHashtable mIdentifierMap; + + nsINode* mAsNode; + const Kind mKind; +}; + +inline const nsTArray* +DocumentOrShadowRoot::GetAllElementsForId(const nsAString& aElementId) const +{ + if (aElementId.IsEmpty()) { + return nullptr; + } + + nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aElementId); + return entry ? &entry->GetIdElements() : nullptr; +} + +} + +} + +#endif diff --git a/dom/base/Element.h b/dom/base/Element.h index 3d9b80a989..fd4fa71084 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -139,6 +139,7 @@ class EventStateManager; namespace dom { +struct CustomElementDefinition; class Animation; class CustomElementRegistry; class Link; diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp index d6c103e8ef..4aab9e435e 100644 --- a/dom/base/ShadowRoot.cpp +++ b/dom/base/ShadowRoot.cpp @@ -58,6 +58,7 @@ ShadowRoot::ShadowRoot(Element* aElement, bool aClosed, already_AddRefed&& aNodeInfo, nsXBLPrototypeBinding* aProtoBinding) : DocumentFragment(aNodeInfo) + , DocumentOrShadowRoot(this) , mProtoBinding(aProtoBinding) , mInsertionPointChanged(false) , mIsComposedDocParticipant(false) @@ -240,7 +241,7 @@ ShadowRoot::InsertSheet(StyleSheet* aSheet, linkingElement->SetStyleSheet(aSheet); // This sets the ownerNode on the sheet - MOZ_DIAGNOSTIC_ASSERT(mProtoBinding->SheetCount() == StyleScope::SheetCount()); + MOZ_DIAGNOSTIC_ASSERT(mProtoBinding->SheetCount() == DocumentOrShadowRoot::SheetCount()); #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED // FIXME(emilio, bug 1425759): For now we keep them duplicated, the proto // binding will disappear soon (tm). @@ -278,44 +279,13 @@ void ShadowRoot::RemoveSheet(StyleSheet* aSheet) { mProtoBinding->RemoveStyleSheet(aSheet); - StyleScope::RemoveSheet(*aSheet); + DocumentOrShadowRoot::RemoveSheet(*aSheet); if (aSheet->IsApplicable()) { StyleSheetChanged(); } } -Element* -ShadowRoot::GetElementById(const nsAString& aElementId) -{ - nsIdentifierMapEntry *entry = mIdentifierMap.GetEntry(aElementId); - return entry ? entry->GetIdElement() : nullptr; -} - -already_AddRefed -ShadowRoot::GetElementsByTagName(const nsAString& aTagName) -{ - return NS_GetContentList(this, kNameSpaceID_Unknown, aTagName); -} - -already_AddRefed -ShadowRoot::GetElementsByTagNameNS(const nsAString& aNamespaceURI, - const nsAString& aLocalName) -{ - int32_t nameSpaceId = kNameSpaceID_Wildcard; - - if (!aNamespaceURI.EqualsLiteral("*")) { - nsresult rv = - nsContentUtils::NameSpaceManager()->RegisterNameSpace(aNamespaceURI, - nameSpaceId); - NS_ENSURE_SUCCESS(rv, nullptr); - } - - NS_ASSERTION(nameSpaceId != kNameSpaceID_Unknown, "Unexpected namespace ID!"); - - return NS_GetContentList(this, nameSpaceId, aLocalName); -} - void ShadowRoot::AddToIdTable(Element* aElement, nsIAtom* aId) { @@ -337,12 +307,6 @@ ShadowRoot::RemoveFromIdTable(Element* aElement, nsIAtom* aId) } } -already_AddRefed -ShadowRoot::GetElementsByClassName(const nsAString& aClasses) -{ - return nsContentUtils::GetElementsByClassName(this, aClasses); -} - nsresult ShadowRoot::GetEventTargetParent(EventChainPreVisitor& aVisitor) { diff --git a/dom/base/ShadowRoot.h b/dom/base/ShadowRoot.h index 21c6e1733d..eb772d49ac 100644 --- a/dom/base/ShadowRoot.h +++ b/dom/base/ShadowRoot.h @@ -8,7 +8,7 @@ #define mozilla_dom_shadowroot_h__ #include "mozilla/dom/DocumentFragment.h" -#include "mozilla/dom/StyleScope.h" +#include "mozilla/dom/DocumentOrShadowRoot.h" #include "nsCOMPtr.h" #include "nsCycleCollectionParticipant.h" #include "nsIContentInlines.h" @@ -29,7 +29,7 @@ namespace dom { class Element; class ShadowRoot final : public DocumentFragment, - public StyleScope, + public DocumentOrShadowRoot, public nsStubMutationObserver { public: @@ -57,22 +57,14 @@ public: return mMode == ShadowRootMode::Closed; } - // StyleScope. - nsINode& AsNode() final - { - return *this; - } - // [deprecated] Shadow DOM v0 - void AddToIdTable(Element* aElement, nsIAtom* aId); - void RemoveFromIdTable(Element* aElement, nsIAtom* aId); void InsertSheet(StyleSheet* aSheet, nsIContent* aLinkingContent); void RemoveSheet(StyleSheet* aSheet); bool ApplyAuthorStyles(); void SetApplyAuthorStyles(bool aApplyAuthorStyles); StyleSheetList* StyleSheets() { - return &StyleScope::EnsureDOMStyleSheets(); + return &DocumentOrShadowRoot::EnsureDOMStyleSheets(); } /** @@ -123,15 +115,11 @@ public: static ShadowRoot* FromNode(nsINode* aNode); + void AddToIdTable(Element* aElement, nsIAtom* aId); + void RemoveFromIdTable(Element* aElement, nsIAtom* aId); + // WebIDL methods. - Element* GetElementById(const nsAString& aElementId); - already_AddRefed - GetElementsByTagName(const nsAString& aNamespaceURI); - already_AddRefed - GetElementsByTagNameNS(const nsAString& aNamespaceURI, - const nsAString& aLocalName); - already_AddRefed - GetElementsByClassName(const nsAString& aClasses); + using mozilla::dom::DocumentOrShadowRoot::GetElementById; void GetInnerHTML(nsAString& aInnerHTML); void SetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError); void StyleSheetChanged(); @@ -154,7 +142,6 @@ protected: // are in the shadow tree and should be kept alive by its parent. nsClassHashtable> mSlotMap; - nsTHashtable mIdentifierMap; nsXBLPrototypeBinding* mProtoBinding; // It is necessary to hold a reference to the associated nsXBLBinding diff --git a/dom/base/StyleScope.cpp b/dom/base/StyleScope.cpp deleted file mode 100644 index 6c708a8ba1..0000000000 --- a/dom/base/StyleScope.cpp +++ /dev/null @@ -1,27 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "StyleScope.h" -#include "mozilla/dom/StyleSheetList.h" - -namespace mozilla { -namespace dom { - -StyleScope::~StyleScope() -{ -} - -StyleSheetList& -StyleScope::EnsureDOMStyleSheets() -{ - if (!mDOMStyleSheets) { - mDOMStyleSheets = new StyleSheetList(*this); - } - return *mDOMStyleSheets; -} - -} -} diff --git a/dom/base/StyleScope.h b/dom/base/StyleScope.h deleted file mode 100644 index c8957b069b..0000000000 --- a/dom/base/StyleScope.h +++ /dev/null @@ -1,81 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef mozilla_dom_StyleScope_h__ -#define mozilla_dom_StyleScope_h__ - -#include "nsTArray.h" - -class nsINode; - -namespace mozilla { -class StyleSheet; - -namespace dom { - -class StyleSheetList; - -/** - * A class meant to be shared by ShadowRoot and Document, that holds a list of - * stylesheets. - * - * TODO(emilio, bug 1418159): In the future this should hold most of the - * relevant style state, this should allow us to fix bug 548397. - */ -class StyleScope -{ -public: - virtual nsINode& AsNode() = 0; - - const nsINode& AsNode() const - { - return const_cast(*this).AsNode(); - } - - StyleSheet* SheetAt(size_t aIndex) const - { - return mStyleSheets.SafeElementAt(aIndex); - } - - size_t SheetCount() const - { - return mStyleSheets.Length(); - } - - int32_t IndexOfSheet(const StyleSheet& aSheet) const - { - return mStyleSheets.IndexOf(&aSheet); - } - - void InsertSheetAt(size_t aIndex, StyleSheet& aSheet) - { - mStyleSheets.InsertElementAt(aIndex, &aSheet); - } - - void RemoveSheet(StyleSheet& aSheet) - { - mStyleSheets.RemoveElement(&aSheet); - } - - void AppendStyleSheet(StyleSheet& aSheet) - { - mStyleSheets.AppendElement(&aSheet); - } - - StyleSheetList& EnsureDOMStyleSheets(); - - ~StyleScope(); - -protected: - nsTArray> mStyleSheets; - RefPtr mDOMStyleSheets; -}; - -} - -} - -#endif diff --git a/dom/base/StyleSheetList.cpp b/dom/base/StyleSheetList.cpp index 42db3179dd..2b41473ffa 100644 --- a/dom/base/StyleSheetList.cpp +++ b/dom/base/StyleSheetList.cpp @@ -48,19 +48,19 @@ StyleSheetList::SlowItem(uint32_t aIndex, nsIDOMStyleSheet** aItem) void StyleSheetList::NodeWillBeDestroyed(const nsINode* aNode) { - mStyleScope = nullptr; + mDocumentOrShadowRoot = nullptr; } -StyleSheetList::StyleSheetList(StyleScope& aScope) - : mStyleScope(&aScope) +StyleSheetList::StyleSheetList(DocumentOrShadowRoot& aScope) + : mDocumentOrShadowRoot(&aScope) { - mStyleScope->AsNode().AddMutationObserver(this); + mDocumentOrShadowRoot->AsNode().AddMutationObserver(this); } StyleSheetList::~StyleSheetList() { - if (mStyleScope) { - mStyleScope->AsNode().RemoveMutationObserver(this); + if (mDocumentOrShadowRoot) { + mDocumentOrShadowRoot->AsNode().RemoveMutationObserver(this); } } diff --git a/dom/base/StyleSheetList.h b/dom/base/StyleSheetList.h index ea5c33a983..f8f371a81b 100644 --- a/dom/base/StyleSheetList.h +++ b/dom/base/StyleSheetList.h @@ -7,7 +7,7 @@ #ifndef mozilla_dom_StyleSheetList_h #define mozilla_dom_StyleSheetList_h -#include "mozilla/dom/StyleScope.h" +#include "mozilla/dom/DocumentOrShadowRoot.h" #include "nsIDOMStyleSheetList.h" #include "nsWrapperCache.h" #include "nsStubDocumentObserver.h" @@ -31,28 +31,28 @@ public: NS_DECL_NSIMUTATIONOBSERVER_NODEWILLBEDESTROYED - explicit StyleSheetList(StyleScope& aScope); + explicit StyleSheetList(DocumentOrShadowRoot& aScope); virtual JSObject* WrapObject(JSContext* aCx, JS::Handle aGivenProto) override final; nsINode* GetParentObject() const { - return mStyleScope ? &mStyleScope->AsNode() : nullptr; + return mDocumentOrShadowRoot ? &mDocumentOrShadowRoot->AsNode() : nullptr; } uint32_t Length() const { - return mStyleScope ? mStyleScope->SheetCount() : 0; + return mDocumentOrShadowRoot ? mDocumentOrShadowRoot->SheetCount() : 0; } StyleSheet* IndexedGetter(uint32_t aIndex, bool& aFound) const { - if (!mStyleScope) { + if (!mDocumentOrShadowRoot) { aFound = false; return nullptr; } - StyleSheet* sheet = mStyleScope->SheetAt(aIndex); + StyleSheet* sheet = mDocumentOrShadowRoot->SheetAt(aIndex); aFound = !!sheet; return sheet; } @@ -66,7 +66,7 @@ public: protected: virtual ~StyleSheetList(); - StyleScope* mStyleScope; // Weak, cleared on "NodeWillBeDestroyed". + DocumentOrShadowRoot* mDocumentOrShadowRoot; // Weak, cleared on "NodeWillBeDestroyed". }; } // namespace dom diff --git a/dom/base/moz.build b/dom/base/moz.build index ab0f0e0abc..cedaa0e49d 100755 --- a/dom/base/moz.build +++ b/dom/base/moz.build @@ -164,6 +164,7 @@ EXPORTS.mozilla.dom += [ 'DirectionalityUtils.h', 'DocGroup.h', 'DocumentFragment.h', + 'DocumentOrShadowRoot.h', 'DocumentType.h', 'DOMCursor.h', 'DOMError.h', @@ -214,7 +215,6 @@ EXPORTS.mozilla.dom += [ 'SimpleTreeIterator.h', 'StructuredCloneHolder.h', 'StructuredCloneTags.h', - 'StyleScope.h', 'StyleSheetList.h', 'SubtleCrypto.h', 'TabGroup.h', @@ -243,6 +243,7 @@ SOURCES += [ 'DirectionalityUtils.cpp', 'DocGroup.cpp', 'DocumentFragment.cpp', + 'DocumentOrShadowRoot.cpp', 'DocumentType.cpp', 'DOMCursor.cpp', 'DOMError.cpp', @@ -361,7 +362,6 @@ SOURCES += [ 'ScriptSettings.cpp', 'ShadowRoot.cpp', 'StructuredCloneHolder.cpp', - 'StyleScope.cpp', 'StyleSheetList.cpp', 'SubtleCrypto.cpp', 'TabGroup.cpp', diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index c0d81a41e0..61d10e0223 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -3658,6 +3658,14 @@ nsContentUtils::ReportToConsole(uint32_t aErrorFlags, aLineNumber, aColumnNumber); } +/* static */ void +nsContentUtils::ReportEmptyGetElementByIdArg(const nsIDocument* aDoc) +{ + ReportToConsole(nsIScriptError::warningFlag, + NS_LITERAL_CSTRING("DOM"), aDoc, + nsContentUtils::eDOM_PROPERTIES, + "EmptyGetElementByIdParam"); +} /* static */ nsresult nsContentUtils::ReportToConsoleNonLocalized(const nsAString& aErrorText, diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index b58b0e0e3c..8cf105bb3e 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -941,6 +941,8 @@ public: uint32_t aLineNumber = 0, uint32_t aColumnNumber = 0); + static void ReportEmptyGetElementByIdArg(const nsIDocument* aDoc); + static void LogMessageToConsole(const char* aMsg); /** diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 203c44cbf4..a1b1408a54 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -300,9 +300,24 @@ GetHttpChannelHelper(nsIChannel* aChannel, nsIHttpChannel** aHttpChannel) #define NAME_NOT_VALID ((nsSimpleContentList*)1) +nsIdentifierMapEntry::nsIdentifierMapEntry(const nsIdentifierMapEntry::AtomOrString& aKey) + : mKey(aKey) +{} + +nsIdentifierMapEntry::nsIdentifierMapEntry(const nsIdentifierMapEntry::AtomOrString* aKey) + : mKey(aKey ? *aKey : nullptr) +{} + nsIdentifierMapEntry::~nsIdentifierMapEntry() -{ -} +{} + +nsIdentifierMapEntry::nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther) + : mKey(mozilla::Move(aOther.mKey)) + , mIdContentList(mozilla::Move(aOther.mIdContentList)) + , mNameContentList(mozilla::Move(aOther.mNameContentList)) + , mChangeCallbacks(mozilla::Move(aOther.mChangeCallbacks)) + , mImageElement(mozilla::Move(aOther.mImageElement)) +{} void nsIdentifierMapEntry::Traverse(nsCycleCollectionTraversalCallback* aCallback) @@ -326,6 +341,12 @@ nsIdentifierMapEntry::IsEmpty() !mChangeCallbacks && !mImageElement; } +bool +nsIdentifierMapEntry::HasNameElement() const +{ + return mNameContentList && mNameContentList->Length() != 0; +} + Element* nsIdentifierMapEntry::GetIdElement() { @@ -1226,6 +1247,7 @@ static already_AddRefed nullNodeInfo; // ================================================================== nsIDocument::nsIDocument() : nsINode(nullNodeInfo), + DocumentOrShadowRoot(this), mReferrerPolicySet(false), mReferrerPolicy(mozilla::net::RP_Default), mBlockAllMixedContent(false), @@ -3234,12 +3256,6 @@ nsDocument::GetElementsByClassName(const nsAString& aClasses, return NS_OK; } -already_AddRefed -nsIDocument::GetElementsByClassName(const nsAString& aClasses) -{ - return nsContentUtils::GetElementsByClassName(this, aClasses); -} - NS_IMETHODIMP nsDocument::ReleaseCapture() { @@ -4729,32 +4745,7 @@ nsDocument::BeginLoad() void nsDocument::ReportEmptyGetElementByIdArg() { - nsContentUtils::ReportToConsole(nsIScriptError::warningFlag, - NS_LITERAL_CSTRING("DOM"), this, - nsContentUtils::eDOM_PROPERTIES, - "EmptyGetElementByIdParam"); -} - -Element* -nsDocument::GetElementById(const nsAString& aElementId) -{ - if (!CheckGetElementByIdArg(aElementId)) { - return nullptr; - } - - nsIdentifierMapEntry *entry = mIdentifierMap.GetEntry(aElementId); - return entry ? entry->GetIdElement() : nullptr; -} - -const nsTArray* -nsDocument::GetAllElementsForId(const nsAString& aElementId) const -{ - if (aElementId.IsEmpty()) { - return nullptr; - } - - nsIdentifierMapEntry *entry = mIdentifierMap.GetEntry(aElementId); - return entry ? &entry->GetIdElements() : nullptr; + nsContentUtils::ReportEmptyGetElementByIdArg(this); } NS_IMETHODIMP @@ -5646,27 +5637,6 @@ nsDocument::BlockedTrackingNodes() const return list.forget(); } -already_AddRefed -nsIDocument::GetElementsByTagNameNS(const nsAString& aNamespaceURI, - const nsAString& aLocalName, - ErrorResult& aResult) -{ - int32_t nameSpaceId = kNameSpaceID_Wildcard; - - if (!aNamespaceURI.EqualsLiteral("*")) { - aResult = - nsContentUtils::NameSpaceManager()->RegisterNameSpace(aNamespaceURI, - nameSpaceId); - if (aResult.Failed()) { - return nullptr; - } - } - - NS_ASSERTION(nameSpaceId != kNameSpaceID_Unknown, "Unexpected namespace ID!"); - - return NS_GetContentList(this, nameSpaceId, aLocalName); -} - NS_IMETHODIMP nsDocument::GetElementsByTagNameNS(const nsAString& aNamespaceURI, const nsAString& aLocalName, @@ -5674,7 +5644,7 @@ nsDocument::GetElementsByTagNameNS(const nsAString& aNamespaceURI, { ErrorResult rv; RefPtr list = - nsIDocument::GetElementsByTagNameNS(aNamespaceURI, aLocalName, rv); + GetElementsByTagNameNS(aNamespaceURI, aLocalName, rv); if (rv.Failed()) { return rv.StealNSResult(); } diff --git a/dom/base/nsDocument.h b/dom/base/nsDocument.h index 6baf40270a..9d7b0aafd1 100644 --- a/dom/base/nsDocument.h +++ b/dom/base/nsDocument.h @@ -333,7 +333,6 @@ class nsDocument : public nsIDocument, public: typedef mozilla::dom::Element Element; - using nsIDocument::GetElementsByTagName; typedef mozilla::net::ReferrerPolicy ReferrerPolicy; NS_DECL_CYCLE_COLLECTING_ISUPPORTS @@ -631,6 +630,11 @@ public: // nsIDOMDocumentXBL NS_DECL_NSIDOMDOCUMENTXBL + using mozilla::dom::DocumentOrShadowRoot::GetElementById; + using mozilla::dom::DocumentOrShadowRoot::GetElementsByTagName; + using mozilla::dom::DocumentOrShadowRoot::GetElementsByTagNameNS; + using mozilla::dom::DocumentOrShadowRoot::GetElementsByClassName; + // nsIDOMEventTarget virtual nsresult GetEventTargetParent( mozilla::EventChainPreVisitor& aVisitor) override; @@ -819,10 +823,7 @@ public: virtual void ResetScrolledToRefAlready() override; virtual void SetChangeScrollPosWhenScrollingToRef(bool aValue) override; - virtual Element *GetElementById(const nsAString& aElementId) override; - virtual const nsTArray* GetAllElementsForId(const nsAString& aElementId) const override; - - virtual Element *LookupImageElement(const nsAString& aElementId) override; + virtual Element* LookupImageElement(const nsAString& aElementId) override; virtual void MozSetImageElement(const nsAString& aImageElementId, Element* aElement) override; @@ -1206,14 +1207,6 @@ public: RefPtr mStyleSheetSetList; RefPtr mScriptLoader; nsDocHeaderData* mHeaderData; - /* mIdentifierMap works as follows for IDs: - * 1) Attribute changes affect the table immediately (removing and adding - * entries as needed). - * 2) Removals from the DOM affect the table immediately - * 3) Additions to the DOM always update existing entries for names, and add - * new ones for IDs. - */ - nsTHashtable mIdentifierMap; nsClassHashtable mRadioGroups; diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index b4fda21c1a..d88db84237 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -34,7 +34,7 @@ #include "prclist.h" #include "mozilla/UniquePtr.h" #include "mozilla/CORSMode.h" -#include "mozilla/dom/StyleScope.h" +#include "mozilla/dom/DocumentOrShadowRoot.h" #include "mozilla/LinkedList.h" #include "mozilla/StyleBackendType.h" #include "mozilla/StyleSheet.h" @@ -198,7 +198,7 @@ class nsContentList; // Document interface. This is implemented by all document objects in // Gecko. class nsIDocument : public nsINode, - public mozilla::dom::StyleScope + public mozilla::dom::DocumentOrShadowRoot { typedef mozilla::dom::GlobalObject GlobalObject; @@ -499,7 +499,7 @@ public: * to remove it. */ typedef bool (* IDTargetObserver)(Element* aOldElement, - Element* aNewelement, void* aData); + Element* aNewelement, void* aData); /** * Add an IDTargetObserver for a specific ID. The IDTargetObserver @@ -1071,14 +1071,9 @@ public: */ virtual void EnsureOnDemandBuiltInUASheet(mozilla::StyleSheet* aSheet) = 0; - nsINode& AsNode() final - { - return *this; - } - mozilla::dom::StyleSheetList* StyleSheets() { - return &StyleScope::EnsureDOMStyleSheets(); + return &DocumentOrShadowRoot::EnsureDOMStyleSheets(); } /** @@ -2362,19 +2357,10 @@ public: virtual void ResetScrolledToRefAlready() = 0; virtual void SetChangeScrollPosWhenScrollingToRef(bool aValue) = 0; - /** - * This method is similar to GetElementById() from nsIDOMDocument but it - * returns a mozilla::dom::Element instead of a nsIDOMElement. - * It prevents converting nsIDOMElement to mozilla::dom::Element which is - * already converted from mozilla::dom::Element. - */ - virtual Element* GetElementById(const nsAString& aElementId) = 0; - - /** - * This method returns _all_ the elements in this document which - * have id aElementId, if there are any. Otherwise it returns null. - */ - virtual const nsTArray* GetAllElementsForId(const nsAString& aElementId) const = 0; + using mozilla::dom::DocumentOrShadowRoot::GetElementById; + using mozilla::dom::DocumentOrShadowRoot::GetElementsByTagName; + using mozilla::dom::DocumentOrShadowRoot::GetElementsByTagNameNS; + using mozilla::dom::DocumentOrShadowRoot::GetElementsByClassName; /** * Lookup an image element using its associated ID, which is usually provided @@ -2574,18 +2560,6 @@ public: nsIDocument* GetTopLevelContentDocument(); - already_AddRefed - GetElementsByTagName(const nsAString& aTagName) - { - return NS_GetContentList(this, kNameSpaceID_Unknown, aTagName); - } - already_AddRefed - GetElementsByTagNameNS(const nsAString& aNamespaceURI, - const nsAString& aLocalName, - mozilla::ErrorResult& aResult); - already_AddRefed - GetElementsByClassName(const nsAString& aClasses); - // GetElementById defined above virtual already_AddRefed CreateElement(const nsAString& aTagName, const mozilla::dom::ElementCreationOptionsOrString& aOptions, diff --git a/dom/base/nsIdentifierMapEntry.h b/dom/base/nsIdentifierMapEntry.h index 41b8b4a836..119a7e4534 100644 --- a/dom/base/nsIdentifierMapEntry.h +++ b/dom/base/nsIdentifierMapEntry.h @@ -15,18 +15,24 @@ #include "mozilla/MemoryReporting.h" #include "mozilla/Move.h" -#include "mozilla/dom/Element.h" #include "mozilla/net/ReferrerPolicy.h" #include "nsCOMArray.h" #include "nsCOMPtr.h" -#include "nsContentList.h" #include "nsIAtom.h" -#include "nsIDocument.h" #include "nsTArray.h" #include "nsTHashtable.h" +#include "nsHashKeys.h" class nsIContent; +class nsContentList; +class nsBaseContentList; + +namespace mozilla { +namespace dom { + class Element; +} // namespace dom +} // namespace mozilla /** * Right now our identifier map entries contain information for 'name' @@ -42,6 +48,16 @@ class nsIContent; */ class nsIdentifierMapEntry : public PLDHashEntryHdr { + typedef mozilla::dom::Element Element; + typedef mozilla::net::ReferrerPolicy ReferrerPolicy; + + /** + * @see nsIDocument::IDTargetObserver, this is just here to avoid include + * hell. + */ + typedef bool (* IDTargetObserver)(Element* aOldElement, + Element* aNewelement, void* aData); + public: struct AtomOrString { @@ -66,25 +82,9 @@ public: typedef const AtomOrString& KeyType; typedef const AtomOrString* KeyTypePointer; - typedef mozilla::dom::Element Element; - typedef mozilla::net::ReferrerPolicy ReferrerPolicy; - - explicit nsIdentifierMapEntry(const AtomOrString& aKey) - : mKey(aKey) - { - } - explicit nsIdentifierMapEntry(const AtomOrString* aKey) - : mKey(aKey ? *aKey : nullptr) - { - } - nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther) : - mKey(mozilla::Move(aOther.GetKey())), - mIdContentList(mozilla::Move(aOther.mIdContentList)), - mNameContentList(aOther.mNameContentList.forget()), - mChangeCallbacks(aOther.mChangeCallbacks.forget()), - mImageElement(aOther.mImageElement.forget()) - { - } + explicit nsIdentifierMapEntry(const AtomOrString& aKey); + explicit nsIdentifierMapEntry(const AtomOrString* aKey); + nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther); ~nsIdentifierMapEntry(); KeyType GetKey() const { return mKey; } @@ -131,9 +131,7 @@ public: nsBaseContentList* GetNameContentList() { return mNameContentList; } - bool HasNameElement() const { - return mNameContentList && mNameContentList->Length() != 0; - } + bool HasNameElement() const; /** * Returns the element if we know the element associated with this @@ -173,9 +171,9 @@ public: bool HasIdElementExposedAsHTMLDocumentProperty(); bool HasContentChangeCallback() { return mChangeCallbacks != nullptr; } - void AddContentChangeCallback(nsIDocument::IDTargetObserver aCallback, + void AddContentChangeCallback(IDTargetObserver aCallback, void* aData, bool aForImage); - void RemoveContentChangeCallback(nsIDocument::IDTargetObserver aCallback, + void RemoveContentChangeCallback(IDTargetObserver aCallback, void* aData, bool aForImage); /** @@ -186,7 +184,7 @@ public: void Traverse(nsCycleCollectionTraversalCallback* aCallback); struct ChangeCallback { - nsIDocument::IDTargetObserver mCallback; + IDTargetObserver mCallback; void* mData; bool mForImage; }; diff --git a/dom/base/nsNameSpaceManager.h b/dom/base/nsNameSpaceManager.h index d5c3a25fe8..e6dc67a3bb 100644 --- a/dom/base/nsNameSpaceManager.h +++ b/dom/base/nsNameSpaceManager.h @@ -10,13 +10,13 @@ #include "nsDataHashtable.h" #include "nsHashKeys.h" #include "nsIAtom.h" -#include "nsIDocument.h" #include "nsIObserver.h" #include "nsTArray.h" #include "mozilla/StaticPtr.h" class nsAString; +class nsIDocument; /** * The Name Space Manager tracks the association between a NameSpace diff --git a/dom/html/nsHTMLDocument.h b/dom/html/nsHTMLDocument.h index c9e46b3faf..4f44befbba 100644 --- a/dom/html/nsHTMLDocument.h +++ b/dom/html/nsHTMLDocument.h @@ -155,10 +155,7 @@ public: virtual void RemovedFromDocShell() override; - virtual mozilla::dom::Element *GetElementById(const nsAString& aElementId) override - { - return nsDocument::GetElementById(aElementId); - } + using mozilla::dom::DocumentOrShadowRoot::GetElementById; virtual void DocAddSizeOfExcludingThis(nsWindowSizes* aWindowSizes) const override; // DocAddSizeOfIncludingThis is inherited from nsIDocument. diff --git a/dom/xul/XULDocument.cpp b/dom/xul/XULDocument.cpp index 929efc1afc..d3b94920a3 100644 --- a/dom/xul/XULDocument.cpp +++ b/dom/xul/XULDocument.cpp @@ -1578,24 +1578,13 @@ XULDocument::GetCommandDispatcher(nsIDOMXULCommandDispatcher** aTracker) } Element* -XULDocument::GetElementById(const nsAString& aId) +XULDocument::GetRefById(const nsAString& aID) { - if (!CheckGetElementByIdArg(aId)) - return nullptr; - - nsIdentifierMapEntry *entry = mIdentifierMap.GetEntry(aId); - if (entry) { - Element* element = entry->GetIdElement(); - if (element) - return element; - } - - nsRefMapEntry* refEntry = mRefMap.GetEntry(aId); - if (refEntry) { - NS_ASSERTION(refEntry->GetFirstElement(), - "nsRefMapEntries should have nonempty content lists"); + if (nsRefMapEntry* refEntry = mRefMap.GetEntry(aID)) { + MOZ_ASSERT(refEntry->GetFirstElement()); return refEntry->GetFirstElement(); } + return nullptr; } diff --git a/dom/xul/XULDocument.h b/dom/xul/XULDocument.h index 06abb797f0..a5ed497049 100644 --- a/dom/xul/XULDocument.h +++ b/dom/xul/XULDocument.h @@ -148,6 +148,7 @@ public: using nsDocument::CreateElementNS; NS_FORWARD_NSIDOMDOCUMENT(XMLDocument::) // And explicitly import the things from nsDocument that we just shadowed + using mozilla::dom::DocumentOrShadowRoot::GetElementById; using nsDocument::GetImplementation; using nsDocument::GetTitle; using nsDocument::SetTitle; @@ -156,8 +157,8 @@ public: using nsDocument::GetMozFullScreenElement; using nsIDocument::GetLocation; - // nsDocument interface overrides - virtual Element* GetElementById(const nsAString & elementId) override; + // Helper for StyleScope::GetElementById. + Element* GetRefById(const nsAString & elementId); // nsIDOMXULDocument interface NS_DECL_NSIDOMXULDOCUMENT diff --git a/editor/libeditor/TextEditor.cpp b/editor/libeditor/TextEditor.cpp index c3cfa4a721..07b06a96aa 100644 --- a/editor/libeditor/TextEditor.cpp +++ b/editor/libeditor/TextEditor.cpp @@ -24,6 +24,7 @@ #include "nsCharTraits.h" #include "nsComponentManagerUtils.h" #include "nsContentCID.h" +#include "nsContentList.h" #include "nsCopySupport.h" #include "nsDebug.h" #include "nsDependentSubstring.h" From 693a79ca08c79f8ca8cc7c825025aaa0eaf9db31 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sat, 13 Jun 2020 08:23:21 -0400 Subject: [PATCH 04/13] Bug 1429656 - Implement ShadowRoot.activeElement Tag #1375 --- dom/base/DocumentOrShadowRoot.cpp | 45 +++++++++++++++++++ dom/base/DocumentOrShadowRoot.h | 9 ++++ dom/base/FragmentOrElement.cpp | 9 ++++ dom/base/ShadowRoot.cpp | 6 +++ dom/base/ShadowRoot.h | 2 + dom/base/nsDocument.cpp | 17 ++----- dom/base/nsFocusManager.cpp | 7 +-- dom/base/nsIContent.h | 8 ++++ dom/webidl/ShadowRoot.webidl | 2 + .../shadow-dom/ShadowRoot-interface.html.ini | 29 ------------ ...activeElement-confirm-return-null.html.ini | 11 ----- .../shadowroot-attributes/test-007.html.ini | 5 --- .../active-element/test-001.html.ini | 5 --- .../active-element/test-002.html.ini | 5 --- 14 files changed, 88 insertions(+), 72 deletions(-) delete mode 100644 testing/web-platform/meta/shadow-dom/ShadowRoot-interface.html.ini delete mode 100644 testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/activeElement-confirm-return-null.html.ini delete mode 100644 testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/test-007.html.ini delete mode 100644 testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-001.html.ini delete mode 100644 testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-002.html.ini diff --git a/dom/base/DocumentOrShadowRoot.cpp b/dom/base/DocumentOrShadowRoot.cpp index 8376ab97e4..13ee3cb150 100644 --- a/dom/base/DocumentOrShadowRoot.cpp +++ b/dom/base/DocumentOrShadowRoot.cpp @@ -6,6 +6,8 @@ #include "DocumentOrShadowRoot.h" #include "mozilla/dom/StyleSheetList.h" +#include "nsDocument.h" +#include "nsFocusManager.h" #include "ShadowRoot.h" #include "XULDocument.h" @@ -100,5 +102,48 @@ DocumentOrShadowRoot::GetElementsByClassName(const nsAString& aClasses) return nsContentUtils::GetElementsByClassName(&AsNode(), aClasses); } +nsIContent* +DocumentOrShadowRoot::Retarget(nsIContent* aContent) const +{ + for (nsIContent* cur = aContent; + cur; + cur = cur->GetContainingShadowHost()) { + if (cur->SubtreeRoot() == &AsNode()) { + return cur; + } + } + return nullptr; +} + +Element* +DocumentOrShadowRoot::GetRetargetedFocusedElement() +{ + if (nsCOMPtr window = AsNode().OwnerDoc()->GetWindow()) { + nsCOMPtr focusedWindow; + nsIContent* focusedContent = + nsFocusManager::GetFocusedDescendant(window, + false, + getter_AddRefs(focusedWindow)); + // be safe and make sure the element is from this document + if (focusedContent && focusedContent->OwnerDoc() == AsNode().OwnerDoc()) { + if (focusedContent->ChromeOnlyAccess()) { + focusedContent = focusedContent->FindFirstNonChromeOnlyAccessContent(); + } + + if (focusedContent) { + if (!nsDocument::IsWebComponentsEnabled(focusedContent)) { + return focusedContent->AsElement(); + } + + if (nsIContent* retarget = Retarget(focusedContent)) { + return retarget->AsElement(); + } + } + } + } + + return nullptr; +} + } } diff --git a/dom/base/DocumentOrShadowRoot.h b/dom/base/DocumentOrShadowRoot.h index 59be0c2a9b..2092cd54f2 100644 --- a/dom/base/DocumentOrShadowRoot.h +++ b/dom/base/DocumentOrShadowRoot.h @@ -116,6 +116,15 @@ public: ~DocumentOrShadowRoot() = default; protected: + nsIContent* Retarget(nsIContent* aContent) const; + + /** + * If focused element's subtree root is this document or shadow root, return + * focused element, otherwise, get the shadow host recursively until the + * shadow host's subtree root is this document or shadow root. + */ + Element* GetRetargetedFocusedElement(); + nsTArray> mStyleSheets; RefPtr mDOMStyleSheets; diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index 6fbe04d251..e33d3c63a4 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -1138,6 +1138,15 @@ FragmentOrElement::GetAssignedSlot() const return slots ? slots->mAssignedSlot.get() : nullptr; } +nsIContent* +nsIContent::GetContainingShadowHost() const +{ + if (mozilla::dom::ShadowRoot* shadow = GetContainingShadow()) { + return shadow->GetHost(); + } + return nullptr; +} + void FragmentOrElement::SetAssignedSlot(HTMLSlotElement* aSlot) { diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp index 4aab9e435e..c354e04c18 100644 --- a/dom/base/ShadowRoot.cpp +++ b/dom/base/ShadowRoot.cpp @@ -461,6 +461,12 @@ ShadowRoot::DistributeAllNodes() DistributionChanged(); } +Element* +ShadowRoot::GetActiveElement() +{ + return GetRetargetedFocusedElement(); +} + void ShadowRoot::GetInnerHTML(nsAString& aInnerHTML) { diff --git a/dom/base/ShadowRoot.h b/dom/base/ShadowRoot.h index eb772d49ac..f83a6f7740 100644 --- a/dom/base/ShadowRoot.h +++ b/dom/base/ShadowRoot.h @@ -120,6 +120,8 @@ public: // WebIDL methods. using mozilla::dom::DocumentOrShadowRoot::GetElementById; + + Element* GetActiveElement(); void GetInnerHTML(nsAString& aInnerHTML); void SetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError); void StyleSheetChanged(); diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index a1b1408a54..40bfa97e2a 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -3031,20 +3031,9 @@ Element* nsIDocument::GetActiveElement() { // Get the focused element. - if (nsCOMPtr window = GetWindow()) { - nsCOMPtr focusedWindow; - nsIContent* focusedContent = - nsFocusManager::GetFocusedDescendant(window, false, - getter_AddRefs(focusedWindow)); - // be safe and make sure the element is from this document - if (focusedContent && focusedContent->OwnerDoc() == this) { - if (focusedContent->ChromeOnlyAccess()) { - focusedContent = focusedContent->FindFirstNonChromeOnlyAccessContent(); - } - if (focusedContent) { - return focusedContent->AsElement(); - } - } + Element* focusedElement = GetRetargetedFocusedElement(); + if (focusedElement) { + return focusedElement; } // No focused element anywhere in this document. Try to get the BODY. diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp index c14087c8a7..3fc9546e84 100644 --- a/dom/base/nsFocusManager.cpp +++ b/dom/base/nsFocusManager.cpp @@ -813,10 +813,11 @@ nsFocusManager::ContentRemoved(nsIDocument* aDocument, nsIContent* aContent) if (!window) return NS_OK; - // if the content is currently focused in the window, or is an ancestor - // of the currently focused element, reset the focus within that window. + // if the content is currently focused in the window, or is an + // shadow-including inclusive ancestor of the currently focused element, + // reset the focus within that window. nsIContent* content = window->GetFocusedNode(); - if (content && nsContentUtils::ContentIsDescendantOf(content, aContent)) { + if (content && nsContentUtils::ContentIsHostIncludingDescendantOf(content, aContent)) { bool shouldShowFocusRing = window->ShouldShowFocusRing(); window->SetFocusedNode(nullptr); diff --git a/dom/base/nsIContent.h b/dom/base/nsIContent.h index ce0a4e5960..101d5e931b 100644 --- a/dom/base/nsIContent.h +++ b/dom/base/nsIContent.h @@ -696,6 +696,14 @@ public: */ virtual mozilla::dom::ShadowRoot *GetContainingShadow() const = 0; + /** + * Gets the shadow host if this content is in a shadow tree. That is, the host + * of |GetContainingShadow|, if its not null. + * + * @return The shadow host, if this is in shadow tree, or null. + */ + nsIContent* GetContainingShadowHost() const; + /** * Gets the assigned slot associated with this content. * diff --git a/dom/webidl/ShadowRoot.webidl b/dom/webidl/ShadowRoot.webidl index 47e6cf5ec5..81b50e58bd 100644 --- a/dom/webidl/ShadowRoot.webidl +++ b/dom/webidl/ShadowRoot.webidl @@ -20,6 +20,8 @@ enum ShadowRootMode { [Func="nsDocument::IsWebComponentsEnabled"] interface ShadowRoot : DocumentFragment { + readonly attribute Element? activeElement; + // Shadow DOM v1 readonly attribute ShadowRootMode mode; readonly attribute Element host; diff --git a/testing/web-platform/meta/shadow-dom/ShadowRoot-interface.html.ini b/testing/web-platform/meta/shadow-dom/ShadowRoot-interface.html.ini deleted file mode 100644 index 546e41144f..0000000000 --- a/testing/web-platform/meta/shadow-dom/ShadowRoot-interface.html.ini +++ /dev/null @@ -1,29 +0,0 @@ -[ShadowRoot-interface.html] - type: testharness - [ShadowRoot.activeElement must return the focused element of the context object when shadow root is open.] - expected: FAIL - - [ShadowRoot.activeElement must return the focused element of the context object when shadow root is closed.] - expected: FAIL - - [ShadowRoot.host must return the shadow host of the context object.] - expected: FAIL - - [ShadowRoot.innerHTML must return the result of the HTML fragment serialization algorithm when shadow root is open.] - expected: FAIL - - [ShadowRoot.innerHTML must return the result of the HTML fragment serialization algorithm when shadow root is closed.] - expected: FAIL - - [ShadowRoot.innerHTML must replace all with the result of invoking the fragment parsing algorithm when shadow root is open.] - expected: FAIL - - [ShadowRoot.innerHTML must replace all with the result of invoking the fragment parsing algorithm when shadow root is closed.] - expected: FAIL - - [ShadowRoot.styleSheets must return a StyleSheetList sequence containing the shadow root style sheets when shadow root is open.] - expected: FAIL - - [ShadowRoot.styleSheets must return a StyleSheetList sequence containing the shadow root style sheets when shadow root is closed.] - expected: FAIL - diff --git a/testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/activeElement-confirm-return-null.html.ini b/testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/activeElement-confirm-return-null.html.ini deleted file mode 100644 index d82555f733..0000000000 --- a/testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/activeElement-confirm-return-null.html.ini +++ /dev/null @@ -1,11 +0,0 @@ -[activeElement-confirm-return-null.html] - type: testharness - [confirm activeElement return null] - expected: FAIL - - [confirm activeElement return null when there is other element in body] - expected: FAIL - - [confirm activeElement return null when focus on the element in the outer shadow tree] - expected: FAIL - diff --git a/testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/test-007.html.ini b/testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/test-007.html.ini deleted file mode 100644 index e621a9779d..0000000000 --- a/testing/web-platform/meta/shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-attributes/test-007.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[test-007.html] - type: testharness - [A_10_01_01_03_01_T01] - expected: FAIL - diff --git a/testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-001.html.ini b/testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-001.html.ini deleted file mode 100644 index fdcb443433..0000000000 --- a/testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-001.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[test-001.html] - type: testharness - [A_07_03_01_T01] - expected: FAIL - diff --git a/testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-002.html.ini b/testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-002.html.ini deleted file mode 100644 index e2a9f3f2f2..0000000000 --- a/testing/web-platform/meta/shadow-dom/untriaged/user-interaction/active-element/test-002.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[test-002.html] - type: testharness - [A_07_03_02_T01] - expected: FAIL - From 90e9fd3550c2f00b9bde18073ffc26ef9bab87eb Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sat, 13 Jun 2020 08:24:18 -0400 Subject: [PATCH 05/13] Bug 1430299 - Add DocumentOrShadowRoot interface Tag #1375 --- dom/webidl/Document.webidl | 4 +--- dom/webidl/DocumentOrShadowRoot.webidl | 29 ++++++++++++++++++++++++++ dom/webidl/ShadowRoot.webidl | 3 --- dom/webidl/moz.build | 1 + 4 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 dom/webidl/DocumentOrShadowRoot.webidl diff --git a/dom/webidl/Document.webidl b/dom/webidl/Document.webidl index ba2ec17ea7..26985202be 100644 --- a/dom/webidl/Document.webidl +++ b/dom/webidl/Document.webidl @@ -136,7 +136,6 @@ partial interface Document { // user interaction [Pure] readonly attribute WindowProxy? defaultView; - readonly attribute Element? activeElement; [Throws] boolean hasFocus(); //(HTML only) attribute DOMString designMode; @@ -283,8 +282,6 @@ partial interface Document { // http://dev.w3.org/csswg/cssom/#extensions-to-the-document-interface partial interface Document { - [Constant] - readonly attribute StyleSheetList styleSheets; attribute DOMString? selectedStyleSheetSet; readonly attribute DOMString? lastStyleSheetSet; readonly attribute DOMString? preferredStyleSheetSet; @@ -456,3 +453,4 @@ Document implements ParentNode; Document implements OnErrorEventHandlerForNodes; Document implements GeometryUtils; Document implements FontFaceSource; +Document implements DocumentOrShadowRoot; diff --git a/dom/webidl/DocumentOrShadowRoot.webidl b/dom/webidl/DocumentOrShadowRoot.webidl new file mode 100644 index 0000000000..16388d1264 --- /dev/null +++ b/dom/webidl/DocumentOrShadowRoot.webidl @@ -0,0 +1,29 @@ +/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. + * + * The origin of this IDL file is + * https://dom.spec.whatwg.org/#documentorshadowroot + * http://w3c.github.io/webcomponents/spec/shadow/#extensions-to-the-documentorshadowroot-mixin + */ + +[NoInterfaceObject] +interface DocumentOrShadowRoot { + // Not implemented yet: bug 1430308. + // Selection? getSelection(); + // Not implemented yet: bug 1430301. + // Element? elementFromPoint (float x, float y); + // Not implemented yet: bug 1430301. + // sequence elementsFromPoint (float x, float y); + // Not implemented yet: bug 1430307. + // CaretPosition? caretPositionFromPoint (float x, float y); + + readonly attribute Element? activeElement; + readonly attribute StyleSheetList styleSheets; + + // Not implemented yet: bug 1430303. + // readonly attribute Element? pointerLockElement; + // Not implemented yet: bug 1430305. + // readonly attribute Element? fullscreenElement; +}; diff --git a/dom/webidl/ShadowRoot.webidl b/dom/webidl/ShadowRoot.webidl index 81b50e58bd..83acd4161d 100644 --- a/dom/webidl/ShadowRoot.webidl +++ b/dom/webidl/ShadowRoot.webidl @@ -20,8 +20,6 @@ enum ShadowRootMode { [Func="nsDocument::IsWebComponentsEnabled"] interface ShadowRoot : DocumentFragment { - readonly attribute Element? activeElement; - // Shadow DOM v1 readonly attribute ShadowRootMode mode; readonly attribute Element host; @@ -34,6 +32,5 @@ interface ShadowRoot : DocumentFragment [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString innerHTML; attribute boolean applyAuthorStyles; - readonly attribute StyleSheetList styleSheets; }; diff --git a/dom/webidl/moz.build b/dom/webidl/moz.build index 2b186bf919..4d0ba78349 100644 --- a/dom/webidl/moz.build +++ b/dom/webidl/moz.build @@ -113,6 +113,7 @@ WEBIDL_FILES = [ 'Directory.webidl', 'Document.webidl', 'DocumentFragment.webidl', + 'DocumentOrShadowRoot.webidl', 'DocumentTimeline.webidl', 'DocumentType.webidl', 'DOMCursor.webidl', From c0409464b6d98b346a9197ec5529ba9e26035c54 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Tue, 9 Jun 2020 23:37:32 -0400 Subject: [PATCH 06/13] Bug 1613623 - For IMAP, no longer allow STARTTLS when server sends PREAUTH greeting. Tag #1312 --- mailnews/imap/src/nsImapProtocol.cpp | 54 ++++++++++++++++++---------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/mailnews/imap/src/nsImapProtocol.cpp b/mailnews/imap/src/nsImapProtocol.cpp index 940d87cbd1..97e61a40ef 100644 --- a/mailnews/imap/src/nsImapProtocol.cpp +++ b/mailnews/imap/src/nsImapProtocol.cpp @@ -1526,28 +1526,44 @@ void nsImapProtocol::EstablishServerConnection() } else if (!PL_strncasecmp(serverResponse, ESC_PREAUTH, ESC_PREAUTH_LEN)) { - // we've been pre-authenticated. - // we can skip the whole password step, right into the - // kAuthenticated state - GetServerStateParser().PreauthSetAuthenticatedState(); + // PREAUTH greeting received. We've been pre-authenticated by the server. + // We can skip sending a password and transition right into the + // kAuthenticated state; but we won't if the user has configured STARTTLS. + // (STARTTLS can only occur with the server in non-authenticated state.) + if (!(m_socketType == nsMsgSocketType::alwaysSTARTTLS || + m_socketType == nsMsgSocketType::trySTARTTLS)) { + GetServerStateParser().PreauthSetAuthenticatedState(); - if (GetServerStateParser().GetCapabilityFlag() == kCapabilityUndefined) - Capability(); + if (GetServerStateParser().GetCapabilityFlag() == kCapabilityUndefined) + Capability(); - if ( !(GetServerStateParser().GetCapabilityFlag() & - (kIMAP4Capability | kIMAP4rev1Capability | kIMAP4other) ) ) - { - // AlertUserEvent_UsingId(MK_MSG_IMAP_SERVER_NOT_IMAP4); + if (!(GetServerStateParser().GetCapabilityFlag() & + (kIMAP4Capability | kIMAP4rev1Capability | kIMAP4other))) { + // AlertUserEventUsingId(MK_MSG_IMAP_SERVER_NOT_IMAP4); + SetConnectionStatus(NS_ERROR_FAILURE); // stop netlib + } else { + // let's record the user as authenticated. + m_imapServerSink->SetUserAuthenticated(true); + + ProcessAfterAuthenticated(); + // the connection was a success + SetConnectionStatus(NS_OK); + } + } else { + // STARTTLS is configured so don't transition to authenticated state. Just + // alert the user, log the error and drop the connection. This may + // indicate a man-in-the middle attack if the user is not expecting + // PREAUTH. The user must change the connection security setting to other + // than STARTTLS to allow PREAUTH to be accepted on subsequent IMAP + // connections. + AlertUserEventUsingName("imapServerDisconnected"); + const nsCString &hostName = GetImapHostName(); + MOZ_LOG( + IMAP, LogLevel::Error, + ("PREAUTH received from IMAP server %s because STARTTLS selected. " + "Connection dropped", + hostName.get())); SetConnectionStatus(NS_ERROR_FAILURE); // stop netlib - } - else - { - // let's record the user as authenticated. - m_imapServerSink->SetUserAuthenticated(true); - - ProcessAfterAuthenticated(); - // the connection was a success - SetConnectionStatus(NS_OK); } } From 478eed8da069112083274e841eb75a2602f1a76f Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 14 Jun 2020 21:43:38 +0000 Subject: [PATCH 07/13] Issue #1594 - restore UAC elevation in the uninstaller. Effectively backs out BZ 1333789 which was uplifted to Firefox before we forked for UXP. Resolves #1594 --- .../mozapps/installer/windows/nsis/common.nsh | 35 ++++--------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/toolkit/mozapps/installer/windows/nsis/common.nsh b/toolkit/mozapps/installer/windows/nsis/common.nsh index 730a30d09a..0016b7eeb5 100644 --- a/toolkit/mozapps/installer/windows/nsis/common.nsh +++ b/toolkit/mozapps/installer/windows/nsis/common.nsh @@ -5151,6 +5151,9 @@ ${GetParameters} $R8 + ; Require elevation if the user can elevate + ${ElevateUAC} + ${If} $R8 != "" ; Default install type StrCpy $InstallType ${INSTALLTYPE_BASIC} @@ -5203,28 +5206,14 @@ FileClose $R5 Delete $R6 ${If} ${Errors} - ; Attempt to elevate and then try again. - ${ElevateUAC} - GetTempFileName $R6 "$INSTDIR" - FileOpen $R5 "$R6" w - FileWrite $R5 "Write Access Test" - FileClose $R5 - Delete $R6 - ${If} ${Errors} - ; Nothing initialized so no need to call OnEndCommon - Quit - ${EndIf} + ; Nothing initialized so no need to call OnEndCommon + Quit ${EndIf} ${Else} CreateDirectory "$INSTDIR" ${If} ${Errors} - ; Attempt to elevate and then try again. - ${ElevateUAC} - CreateDirectory "$INSTDIR" - ${If} ${Errors} - ; Nothing initialized so no need to call OnEndCommon - Quit - ${EndIf} + ; Nothing initialized so no need to call OnEndCommon + Quit ${EndIf} ${EndIf} @@ -5256,20 +5245,10 @@ ${EndIf} !endif ${EndIf} - ${Else} - ; If this isn't an INI install, we need to try to elevate now. - ; We'll check the user's permission level later on to determine the - ; default install path (which will be the real install path for /S). - ; If an INI file is used, we try to elevate down that path when needed. - ${ElevateUAC} ${EndUnless} ${EndIf} ClearErrors - ${IfNot} ${Silent} - ${ElevateUAC} - ${EndIf} - Pop $R5 Pop $R6 Pop $R7 From 2b8d258df0b40b5e000f69e04a1378d2f50f3509 Mon Sep 17 00:00:00 2001 From: Lootyhoof Date: Sun, 14 Jun 2020 23:23:23 +0100 Subject: [PATCH 08/13] Follow up to Issue #1578 - Remove unnecessary MOZ_ASSERT --- widget/gtk/nsNativeMenuService.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/widget/gtk/nsNativeMenuService.cpp b/widget/gtk/nsNativeMenuService.cpp index eabe77ca80..7b92e73e8f 100644 --- a/widget/gtk/nsNativeMenuService.cpp +++ b/widget/gtk/nsNativeMenuService.cpp @@ -160,7 +160,6 @@ nsNativeMenuService::~nsNativeMenuService() { gPangoLayout = nullptr; } - MOZ_ASSERT(sService == this); sService = nullptr; } From bdd709df2b0c8a6e8363c4459c4a47b67bc03565 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 11:02:24 -0400 Subject: [PATCH 09/13] Bug 1316302 - Part 1: Helper methods for HTMLEditRules::WillDeleteSelection() should have an out argument to indicates if it actually handles the action When HTMLEditRules::WillDeleteSelection() tries to remove something from the end/start of a block to its last/first text node but it's contained by block elements, it tries to join the container and the block. However, JoinBlocks() always fails to join them since it's impossible operation. In this case, HTMLEditRules::WillDeleteSelection() should retry to remove something in the leaf, however, it's impossible for now because JoinBlocks() and its helper methods don't return if it handles the action actually. This patch renames |JoinBlocks()| to |TryToJoinBlocks()| for representing what it is. And this patch adds |bool* aHandled| to the helper methods. Then, *aHandled and *aCancel are now always returns the result of each method. Therefore, for merging the result of multiple helper methods, callers need to receive the result with temporary variables and merge them by themselves. Note that when they modify DOM node actually or the action should do nothing (for example, selection is across tables), aHandled is set to true. Tag #1563 --- editor/libeditor/HTMLEditRules.cpp | 142 ++++++++++++++++++----------- editor/libeditor/HTMLEditRules.h | 66 +++++++++++++- 2 files changed, 149 insertions(+), 59 deletions(-) diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index 805092eb78..5cd370f133 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -1844,10 +1844,10 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, // origCollapsed is used later to determine whether we should join blocks. We // don't really care about bCollapsed because it will be modified by - // ExtendSelectionForDelete later. JoinBlocks should happen if the original - // selection is collapsed and the cursor is at the end of a block element, in - // which case ExtendSelectionForDelete would always make the selection not - // collapsed. + // ExtendSelectionForDelete later. TryToJoinBlocks() should happen if the + // original selection is collapsed and the cursor is at the end of a block + // element, in which case ExtendSelectionForDelete would always make the + // selection not collapsed. bool bCollapsed = aSelection->Collapsed(); bool join = false; bool origCollapsed = bCollapsed; @@ -2196,9 +2196,13 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, address_of(selPointNode), &selPointOffset); NS_ENSURE_STATE(leftNode && leftNode->IsContent() && rightNode && rightNode->IsContent()); + bool handled = false, canceled = false; + rv = TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(), + &canceled, &handled); + // TODO: If it does nothing and previous or next node is a text node, + // we should modify it. *aHandled = true; - rv = JoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(), - aCancel); + *aCancel |= canceled; NS_ENSURE_SUCCESS(rv, rv); } aSelection->Collapse(selPointNode, selPointOffset); @@ -2247,9 +2251,14 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater, address_of(selPointNode), &selPointOffset); NS_ENSURE_STATE(leftNode->IsContent() && rightNode->IsContent()); + bool handled = false, canceled = false; + rv = TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(), + &canceled, &handled); + // This should claim that trying to join the block means that + // this handles the action because the caller shouldn't do anything + // anymore in this case. *aHandled = true; - rv = JoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(), - aCancel); + *aCancel |= canceled; NS_ENSURE_SUCCESS(rv, rv); } aSelection->Collapse(selPointNode, selPointOffset); @@ -2421,7 +2430,10 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, } if (join) { - rv = JoinBlocks(*leftParent, *rightParent, aCancel); + bool handled = false, canceled = false; + rv = TryToJoinBlocks(*leftParent, *rightParent, &canceled, &handled); + MOZ_ASSERT(*aHandled); + *aCancel |= canceled; NS_ENSURE_SUCCESS(rv, rv); } } @@ -2571,21 +2583,17 @@ HTMLEditRules::GetGoodSelPointForNode(nsINode& aNode, return ret; } - -/** - * This method is used to join two block elements. The right element is always - * joined to the left element. If the elements are the same type and not - * nested within each other, JoinNodesSmart is called (example, joining two - * list items together into one). If the elements are not the same type, or - * one is a descendant of the other, we instead destroy the right block placing - * its children into leftblock. DTD containment rules are followed throughout. - */ nsresult -HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, - nsIContent& aRightNode, - bool* aCanceled) +HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, + nsIContent& aRightNode, + bool* aCanceled, + bool* aHandled) { MOZ_ASSERT(aCanceled); + MOZ_ASSERT(aHandled); + + *aCanceled = false; + *aHandled = false; NS_ENSURE_STATE(mHTMLEditor); RefPtr htmlEditor(mHTMLEditor); @@ -2601,6 +2609,7 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, HTMLEditUtils::IsTableElement(rightBlock)) { // Do not try to merge table elements *aCanceled = true; + *aHandled = true; return NS_OK; } @@ -2617,6 +2626,7 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, // Bail if both blocks the same if (leftBlock == rightBlock) { *aCanceled = true; + *aHandled = true; return NS_OK; } @@ -2624,6 +2634,7 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, if (HTMLEditUtils::IsList(leftBlock) && HTMLEditUtils::IsListItem(rightBlock) && rightBlock->GetParentNode() == leftBlock) { + *aHandled = true; return NS_OK; } @@ -2694,11 +2705,15 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, rv = htmlEditor->MoveNode(child, leftList, -1); NS_ENSURE_SUCCESS(rv, rv); } + // XXX Should this set to true only when above for loop moves the node? + *aHandled = true; } else { - MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset); + bool handled = false; + MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset, &handled); + *aHandled |= handled; } - if (brNode) { - htmlEditor->DeleteNode(brNode); + if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { + *aHandled = true; } // Offset below is where you find yourself in leftBlock when you traverse // upwards from rightBlock @@ -2730,7 +2745,9 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, nsCOMPtr brNode = CheckForInvisibleBR(*leftBlock, BRLocation::beforeBlock, leftOffset); if (mergeLists) { - MoveContents(*rightList, *leftList, &leftOffset); + bool handled = false; + MoveContents(*rightList, *leftList, &leftOffset, &handled); + *aHandled |= handled; } else { // Left block is a parent of right block, and the parent of the previous // visible content. Right block is a child and contains the contents we @@ -2786,12 +2803,14 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, NS_ENSURE_TRUE(previousContentParent, NS_ERROR_NULL_POINTER); + bool handled = false; rv = MoveBlock(*previousContentParent->AsElement(), *rightBlock, - previousContentOffset, rightOffset); + previousContentOffset, rightOffset, &handled); + *aHandled |= handled; NS_ENSURE_SUCCESS(rv, rv); } - if (brNode) { - htmlEditor->DeleteNode(brNode); + if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { + *aHandled = true; } } else { // Normal case. Blocks are siblings, or at least close enough. An example @@ -2815,32 +2834,37 @@ HTMLEditRules::JoinBlocks(nsIContent& aLeftNode, ConvertListType(rightBlock, getter_AddRefs(newBlock), existingList, nsGkAtoms::li); } + *aHandled = true; } else { // Nodes are dissimilar types. - rv = MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset); + bool handled = false; + rv = + MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset, &handled); + *aHandled |= handled; NS_ENSURE_SUCCESS(rv, rv); } if (brNode) { rv = htmlEditor->DeleteNode(brNode); + // XXX In other top level if/else-if blocks, the result of DeleteNode() + // is ignored. Why does only this result is respected? NS_ENSURE_SUCCESS(rv, rv); + *aHandled = true; } } return NS_OK; } - -/** - * Moves the content from aRightBlock starting from aRightOffset into - * aLeftBlock at aLeftOffset. Note that the "block" might merely be inline - * nodes between
s, or between blocks, etc. DTD containment rules are - * followed throughout. - */ nsresult HTMLEditRules::MoveBlock(Element& aLeftBlock, Element& aRightBlock, int32_t aLeftOffset, - int32_t aRightOffset) + int32_t aRightOffset, + bool* aHandled) { + MOZ_ASSERT(aHandled); + + *aHandled = false; + nsTArray> arrayOfNodes; // GetNodesFromPoint is the workhorse that figures out what we wnat to move. nsresult rv = GetNodesFromPoint(EditorDOMPoint(&aRightBlock, aRightOffset), @@ -2851,15 +2875,20 @@ HTMLEditRules::MoveBlock(Element& aLeftBlock, // get the node to act on if (IsBlockNode(arrayOfNodes[i])) { // For block nodes, move their contents only, then delete block. + bool handled = false; rv = MoveContents(*arrayOfNodes[i]->AsElement(), aLeftBlock, - &aLeftOffset); + &aLeftOffset, &handled); + *aHandled |= handled; NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_STATE(mHTMLEditor); rv = mHTMLEditor->DeleteNode(arrayOfNodes[i]); + *aHandled = true; } else { // Otherwise move the content as is, checking against the DTD. + bool handled = false; rv = MoveNodeSmart(*arrayOfNodes[i]->AsContent(), aLeftBlock, - &aLeftOffset); + &aLeftOffset, &handled); + *aHandled |= handled; } } @@ -2868,17 +2897,16 @@ HTMLEditRules::MoveBlock(Element& aLeftBlock, return NS_OK; } -/** - * This method is used to move node aNode to (aDestElement, aInOutDestOffset). - * DTD containment rules are followed throughout. aInOutDestOffset is updated - * to point _after_ inserted content. - */ nsresult HTMLEditRules::MoveNodeSmart(nsIContent& aNode, Element& aDestElement, - int32_t* aInOutDestOffset) + int32_t* aInOutDestOffset, + bool* aHandled) { MOZ_ASSERT(aInOutDestOffset); + MOZ_ASSERT(aHandled); + + *aHandled = false; NS_ENSURE_STATE(mHTMLEditor); RefPtr htmlEditor(mHTMLEditor); @@ -2892,37 +2920,43 @@ HTMLEditRules::MoveNodeSmart(nsIContent& aNode, if (*aInOutDestOffset != -1) { (*aInOutDestOffset)++; } + // XXX Should we check if the node is actually moved in this case? + *aHandled = true; } else { // If it can't, move its children (if any), and then delete it. if (aNode.IsElement()) { - nsresult rv = - MoveContents(*aNode.AsElement(), aDestElement, aInOutDestOffset); + bool handled = false; + nsresult rv = MoveContents(*aNode.AsElement(), aDestElement, + aInOutDestOffset, &handled); + *aHandled |= handled; NS_ENSURE_SUCCESS(rv, rv); } nsresult rv = htmlEditor->DeleteNode(&aNode); NS_ENSURE_SUCCESS(rv, rv); + *aHandled = true; } return NS_OK; } -/** - * Moves the _contents_ of aElement to (aDestElement, aInOutDestOffset). DTD - * containment rules are followed throughout. aInOutDestOffset is updated to - * point _after_ inserted content. - */ nsresult HTMLEditRules::MoveContents(Element& aElement, Element& aDestElement, - int32_t* aInOutDestOffset) + int32_t* aInOutDestOffset, + bool* aHandled) { MOZ_ASSERT(aInOutDestOffset); + MOZ_ASSERT(aHandled); + + *aHandled = false; NS_ENSURE_TRUE(&aElement != &aDestElement, NS_ERROR_ILLEGAL_VALUE); while (aElement.GetFirstChild()) { + bool handled = false; nsresult rv = MoveNodeSmart(*aElement.GetFirstChild(), aDestElement, - aInOutDestOffset); + aInOutDestOffset, &handled); + *aHandled |= handled; NS_ENSURE_SUCCESS(rv, rv); } return NS_OK; diff --git a/editor/libeditor/HTMLEditRules.h b/editor/libeditor/HTMLEditRules.h index 40c5e2afde..6cdfa57cf1 100644 --- a/editor/libeditor/HTMLEditRules.h +++ b/editor/libeditor/HTMLEditRules.h @@ -163,14 +163,70 @@ protected: nsresult InsertBRIfNeeded(Selection* aSelection); mozilla::EditorDOMPoint GetGoodSelPointForNode(nsINode& aNode, nsIEditor::EDirection aAction); - nsresult JoinBlocks(nsIContent& aLeftNode, nsIContent& aRightNode, - bool* aCanceled); + + /** + * TryToJoinBlocks() tries to join two block elements. The right element is + * always joined to the left element. If the elements are the same type and + * not nested within each other, JoinNodesSmart() is called (example, joining + * two list items together into one). If the elements are not the same type, + * or one is a descendant of the other, we instead destroy the right block + * placing its children into leftblock. DTD containment rules are followed + * throughout. + * + * @param aCanceled returns true if the operation should do nothing anymore + * even if this doesn't join the blocks. + * NOTE: When this returns an error, nobody should refer + * the result of this. + * @param aHandled returns true if this actually handles the request. + * Note that this may return true even if this does not + * join the block. E.g., if the blocks shouldn't be + * joined or it's impossible to join them but it's not + * unexpected case, this returns true with this. + * NOTE: When this returns an error, nobody should refer + * the result of this. + */ + nsresult TryToJoinBlocks(nsIContent& aLeftNode, nsIContent& aRightNode, + bool* aCanceled, bool* aHandled); + + /** + * MoveBlock() moves the content from aRightBlock starting from aRightOffset + * into aLeftBlock at aLeftOffset. Note that the "block" can be inline nodes + * between
s, or between blocks, etc. DTD containment rules are followed + * throughout. + * + * @param aHandled returns true if this actually joins the nodes. + * NOTE: When this returns an error, nobody should refer + * the result of this. + */ nsresult MoveBlock(Element& aLeftBlock, Element& aRightBlock, - int32_t aLeftOffset, int32_t aRightOffset); + int32_t aLeftOffset, int32_t aRightOffset, + bool* aHandled); + + /** + * MoveNodeSmart() moves aNode to (aDestElement, aInOutDestOffset). + * DTD containment rules are followed throughout. + * + * @param aOffset returns the point after inserted content. + * @param aHandled returns true if this actually moves the + * nodes. + * NOTE: When this returns an error, nobody + * should refer the result of this. + */ nsresult MoveNodeSmart(nsIContent& aNode, Element& aDestElement, - int32_t* aOffset); + int32_t* aInOutDestOffset, bool* aHandled); + + /** + * MoveContents() moves the contents of aElement to (aDestElement, + * aInOutDestOffset). DTD containment rules are followed throughout. + * + * @param aInOutDestOffset updated to point after inserted content. + * @param aHandled returns true if this actually moves the + * nodes. + * NOTE: When this returns an error, nobody + * should refer the result of this. + */ nsresult MoveContents(Element& aElement, Element& aDestElement, - int32_t* aOffset); + int32_t* aInOutDestOffset, bool* aHandled); nsresult DeleteNonTableElements(nsINode* aNode); nsresult WillMakeList(Selection* aSelection, const nsAString* aListType, From 2afdd9f0ba9f9a2ed19ba8705691545304caf59d Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 11:03:43 -0400 Subject: [PATCH 10/13] Bug 1316302 - Part 2: WillDeleteSelection() should retry to handle it when selection is collapsed and JoinBlocks() doesn't handle nor cancel the action When selection is collapsed and JoinBlocks() doesn't handle nor cancel the action, WillDeleteSelection() should move selection to the start/end of leftmost/rightmost editable leaf node and retry to handle the action again. For avoiding infinite loop, it checks if selected node is changed actually before calling itself again. Tag #1563 --- editor/libeditor/HTMLEditRules.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index 5cd370f133..3a773dc3fb 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -2199,12 +2199,24 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, bool handled = false, canceled = false; rv = TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(), &canceled, &handled); - // TODO: If it does nothing and previous or next node is a text node, - // we should modify it. - *aHandled = true; + *aHandled |= handled; *aCancel |= canceled; NS_ENSURE_SUCCESS(rv, rv); } + + // If TryToJoinBlocks() didn't handle it and it's not canceled, + // user may want to modify the start leaf node or the last leaf node + // of the block. + if (!*aHandled && !*aCancel && leafNode != startNode) { + int32_t offset = + aAction == nsIEditor::ePrevious ? + static_cast(leafNode->Length()) : 0; + aSelection->Collapse(leafNode, offset); + return WillDeleteSelection(aSelection, aAction, aStripWrappers, + aCancel, aHandled); + } + + // Otherwise, we must have deleted the selection as user expected. aSelection->Collapse(selPointNode, selPointOffset); return NS_OK; } From 4731db5173a153fb670c7f35433dd23f1b17d7d6 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 11:48:40 -0400 Subject: [PATCH 11/13] Bug 1316302 - Part 3: Create EditActionResult class for making the methods which return nsresult, handled and canceled with out params In a lot of places, edit action handlers and their helper methods return nsresult and aHandled and aCanceled with out params. However, the out params cause the code complicated since: * it's not unclear if the method will overwrite aHandled and aCanceled value. * callers need to create temporary variable event if some of them are not necessary. This patch rewrites the helper methods of HTMLEditRules::WillDeleteSelection() with it. Tag #1563 --- editor/libeditor/EditorUtils.h | 113 ++++++++++++ editor/libeditor/HTMLEditRules.cpp | 278 ++++++++++++++++------------- editor/libeditor/HTMLEditRules.h | 54 +++--- 3 files changed, 294 insertions(+), 151 deletions(-) diff --git a/editor/libeditor/EditorUtils.h b/editor/libeditor/EditorUtils.h index 34286da8a9..15ec0b62db 100644 --- a/editor/libeditor/EditorUtils.h +++ b/editor/libeditor/EditorUtils.h @@ -30,6 +30,119 @@ namespace dom { class Selection; } // namespace dom +/*************************************************************************** + * EditActionResult is useful to return multiple results of an editor + * action handler without out params. + * Note that when you return an anonymous instance from a method, you should + * use EditActionIgnored(), EditActionHandled() or EditActionCanceled() for + * easier to read. In other words, EditActionResult should be used when + * declaring return type of a method, being an argument or defined as a local + * variable. + */ +class MOZ_STACK_CLASS EditActionResult final +{ +public: + bool Succeeded() const { return NS_SUCCEEDED(mRv); } + bool Failed() const { return NS_FAILED(mRv); } + nsresult Rv() const { return mRv; } + bool Canceled() const { return mCanceled; } + bool Handled() const { return mHandled; } + + EditActionResult SetResult(nsresult aRv) + { + mRv = aRv; + return *this; + } + EditActionResult MarkAsCanceled() + { + mCanceled = true; + return *this; + } + EditActionResult MarkAsHandled() + { + mHandled = true; + return *this; + } + + explicit EditActionResult(nsresult aRv) + : mRv(aRv) + , mCanceled(false) + , mHandled(false) + { + } + + EditActionResult& operator|=(const EditActionResult& aOther) + { + mCanceled |= aOther.mCanceled; + mHandled |= aOther.mHandled; + // When both result are same, keep the result. + if (mRv == aOther.mRv) { + return *this; + } + // If one of the results is error, use NS_ERROR_FAILURE. + if (Failed() || aOther.Failed()) { + mRv = NS_ERROR_FAILURE; + } else { + // Otherwise, use generic success code, NS_OK. + mRv = NS_OK; + } + return *this; + } + +private: + nsresult mRv; + bool mCanceled; + bool mHandled; + + EditActionResult(nsresult aRv, bool aCanceled, bool aHandled) + : mRv(aRv) + , mCanceled(aCanceled) + , mHandled(aHandled) + { + } + + EditActionResult() + : mRv(NS_ERROR_NOT_INITIALIZED) + , mCanceled(false) + , mHandled(false) + { + } + + friend EditActionResult EditActionIgnored(nsresult aRv); + friend EditActionResult EditActionHandled(nsresult aRv); + friend EditActionResult EditActionCanceled(nsresult aRv); +}; + +/*************************************************************************** + * When an edit action handler (or its helper) does nothing, + * EditActionIgnored should be returned. + */ +inline EditActionResult +EditActionIgnored(nsresult aRv = NS_OK) +{ + return EditActionResult(aRv, false, false); +} + +/*************************************************************************** + * When an edit action handler (or its helper) handled and not canceled, + * EditActionHandled should be returned. + */ +inline EditActionResult +EditActionHandled(nsresult aRv = NS_OK) +{ + return EditActionResult(aRv, false, true); +} + +/*************************************************************************** + * When an edit action handler (or its helper) handled and canceled, + * EditActionHandled should be returned. + */ +inline EditActionResult +EditActionCanceled(nsresult aRv = NS_OK) +{ + return EditActionResult(aRv, true, true); +} + /*************************************************************************** * stack based helper class for batching a collection of txns inside a * placeholder txn. diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index 3a773dc3fb..ef5456f62e 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -2196,12 +2196,13 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, address_of(selPointNode), &selPointOffset); NS_ENSURE_STATE(leftNode && leftNode->IsContent() && rightNode && rightNode->IsContent()); - bool handled = false, canceled = false; - rv = TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(), - &canceled, &handled); - *aHandled |= handled; - *aCancel |= canceled; - NS_ENSURE_SUCCESS(rv, rv); + EditActionResult ret = + TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent()); + *aHandled |= ret.Handled(); + *aCancel |= ret.Canceled(); + if (NS_WARN_IF(ret.Failed())) { + return ret.Rv(); + } } // If TryToJoinBlocks() didn't handle it and it's not canceled, @@ -2263,15 +2264,16 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater, address_of(selPointNode), &selPointOffset); NS_ENSURE_STATE(leftNode->IsContent() && rightNode->IsContent()); - bool handled = false, canceled = false; - rv = TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent(), - &canceled, &handled); + EditActionResult ret = + TryToJoinBlocks(*leftNode->AsContent(), *rightNode->AsContent()); // This should claim that trying to join the block means that // this handles the action because the caller shouldn't do anything // anymore in this case. *aHandled = true; - *aCancel |= canceled; - NS_ENSURE_SUCCESS(rv, rv); + *aCancel |= ret.Canceled(); + if (NS_WARN_IF(ret.Failed())) { + return ret.Rv(); + } } aSelection->Collapse(selPointNode, selPointOffset); return NS_OK; @@ -2442,11 +2444,12 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, } if (join) { - bool handled = false, canceled = false; - rv = TryToJoinBlocks(*leftParent, *rightParent, &canceled, &handled); + EditActionResult ret = TryToJoinBlocks(*leftParent, *rightParent); MOZ_ASSERT(*aHandled); - *aCancel |= canceled; - NS_ENSURE_SUCCESS(rv, rv); + *aCancel |= ret.Canceled(); + if (NS_WARN_IF(ret.Failed())) { + return ret.Rv(); + } } } } @@ -2595,59 +2598,58 @@ HTMLEditRules::GetGoodSelPointForNode(nsINode& aNode, return ret; } -nsresult +EditActionResult HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, - nsIContent& aRightNode, - bool* aCanceled, - bool* aHandled) + nsIContent& aRightNode) { - MOZ_ASSERT(aCanceled); - MOZ_ASSERT(aHandled); + if (NS_WARN_IF(!mHTMLEditor)) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } - *aCanceled = false; - *aHandled = false; - - NS_ENSURE_STATE(mHTMLEditor); RefPtr htmlEditor(mHTMLEditor); nsCOMPtr leftBlock = htmlEditor->GetBlock(aLeftNode); nsCOMPtr rightBlock = htmlEditor->GetBlock(aRightNode); // Sanity checks - NS_ENSURE_TRUE(leftBlock && rightBlock, NS_ERROR_NULL_POINTER); - NS_ENSURE_STATE(leftBlock != rightBlock); + if (NS_WARN_IF(!leftBlock) || NS_WARN_IF(!rightBlock)) { + return EditActionIgnored(NS_ERROR_NULL_POINTER); + } + if (NS_WARN_IF(leftBlock == rightBlock)) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } if (HTMLEditUtils::IsTableElement(leftBlock) || HTMLEditUtils::IsTableElement(rightBlock)) { // Do not try to merge table elements - *aCanceled = true; - *aHandled = true; - return NS_OK; + return EditActionCanceled(); } // Make sure we don't try to move things into HR's, which look like blocks // but aren't containers if (leftBlock->IsHTMLElement(nsGkAtoms::hr)) { leftBlock = htmlEditor->GetBlockNodeParent(leftBlock); + if (NS_WARN_IF(!leftBlock)) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } } if (rightBlock->IsHTMLElement(nsGkAtoms::hr)) { rightBlock = htmlEditor->GetBlockNodeParent(rightBlock); + if (NS_WARN_IF(!rightBlock)) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } } - NS_ENSURE_STATE(leftBlock && rightBlock); // Bail if both blocks the same if (leftBlock == rightBlock) { - *aCanceled = true; - *aHandled = true; - return NS_OK; + return EditActionIgnored(); } // Joining a list item to its parent is a NOP. if (HTMLEditUtils::IsList(leftBlock) && HTMLEditUtils::IsListItem(rightBlock) && rightBlock->GetParentNode() == leftBlock) { - *aHandled = true; - return NS_OK; + return EditActionHandled(); } // Special rule here: if we are trying to join list items, and they are in @@ -2681,6 +2683,7 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, // offset below is where you find yourself in rightBlock when you traverse // upwards from leftBlock + EditActionResult ret(NS_OK); if (EditorUtils::IsDescendantOf(leftBlock, rightBlock, &rightOffset)) { // Tricky case. Left block is inside right block. Do ws adjustment. This // just destroys non-visible ws at boundaries we will be joining. @@ -2688,7 +2691,9 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, nsresult rv = WSRunObject::ScrubBlockBoundary(htmlEditor, WSRunObject::kBlockEnd, leftBlock); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } { // We can't just track rightBlock because it's an Element. @@ -2698,11 +2703,16 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, rv = WSRunObject::ScrubBlockBoundary(htmlEditor, WSRunObject::kAfterBlock, rightBlock, rightOffset); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } + if (trackingRightBlock->IsElement()) { rightBlock = trackingRightBlock->AsElement(); } else { - NS_ENSURE_STATE(trackingRightBlock->GetParentElement()); + if (NS_WARN_IF(!trackingRightBlock->GetParentElement())) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } rightBlock = trackingRightBlock->GetParentElement(); } } @@ -2715,17 +2725,22 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, for (nsCOMPtr child = rightList->GetChildAt(offset); child; child = rightList->GetChildAt(rightOffset)) { rv = htmlEditor->MoveNode(child, leftList, -1); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } } // XXX Should this set to true only when above for loop moves the node? - *aHandled = true; + ret.MarkAsHandled(); } else { - bool handled = false; - MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset, &handled); - *aHandled |= handled; + // XXX Why do we ignore the result of MoveBlock()? + EditActionResult retMoveBlock = + MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset); + if (retMoveBlock.Handled()) { + ret.MarkAsHandled(); + } } if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { - *aHandled = true; + ret.MarkAsHandled(); } // Offset below is where you find yourself in leftBlock when you traverse // upwards from rightBlock @@ -2735,7 +2750,10 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, nsresult rv = WSRunObject::ScrubBlockBoundary(htmlEditor, WSRunObject::kBlockStart, rightBlock); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } + { // We can't just track leftBlock because it's an Element, so track // something else. @@ -2745,11 +2763,16 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, rv = WSRunObject::ScrubBlockBoundary(htmlEditor, WSRunObject::kBeforeBlock, leftBlock, leftOffset); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } + if (trackingLeftBlock->IsElement()) { leftBlock = trackingLeftBlock->AsElement(); } else { - NS_ENSURE_STATE(trackingLeftBlock->GetParentElement()); + if (NS_WARN_IF(!trackingLeftBlock->GetParentElement())) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } leftBlock = trackingLeftBlock->GetParentElement(); } } @@ -2757,9 +2780,12 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, nsCOMPtr brNode = CheckForInvisibleBR(*leftBlock, BRLocation::beforeBlock, leftOffset); if (mergeLists) { - bool handled = false; - MoveContents(*rightList, *leftList, &leftOffset, &handled); - *aHandled |= handled; + // XXX Why do we ignore the result of MoveContents()? + EditActionResult retMoveContents = + MoveContents(*rightList, *leftList, &leftOffset); + if (retMoveContents.Handled()) { + ret.MarkAsHandled(); + } } else { // Left block is a parent of right block, and the parent of the previous // visible content. Right block is a child and contains the contents we @@ -2804,7 +2830,9 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, &previousContentOffset, nullptr, nullptr, nullptr, getter_AddRefs(splittedPreviousContent)); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } if (splittedPreviousContent) { previousContentParent = splittedPreviousContent->GetParentNode(); @@ -2813,16 +2841,18 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, } } - NS_ENSURE_TRUE(previousContentParent, NS_ERROR_NULL_POINTER); + if (NS_WARN_IF(!previousContentParent)) { + return EditActionIgnored(NS_ERROR_NULL_POINTER); + } - bool handled = false; - rv = MoveBlock(*previousContentParent->AsElement(), *rightBlock, - previousContentOffset, rightOffset, &handled); - *aHandled |= handled; - NS_ENSURE_SUCCESS(rv, rv); + ret |= MoveBlock(*previousContentParent->AsElement(), *rightBlock, + previousContentOffset, rightOffset); + if (NS_WARN_IF(ret.Failed())) { + return ret; + } } if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { - *aHandled = true; + ret.MarkAsHandled(); } } else { // Normal case. Blocks are siblings, or at least close enough. An example @@ -2833,7 +2863,9 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, // Adjust whitespace at block boundaries nsresult rv = WSRunObject::PrepareToJoinBlocks(htmlEditor, leftBlock, rightBlock); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } // Do br adjustment. nsCOMPtr brNode = CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd); @@ -2846,81 +2878,83 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, ConvertListType(rightBlock, getter_AddRefs(newBlock), existingList, nsGkAtoms::li); } - *aHandled = true; + ret.MarkAsHandled(); } else { // Nodes are dissimilar types. - bool handled = false; - rv = - MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset, &handled); - *aHandled |= handled; - NS_ENSURE_SUCCESS(rv, rv); + ret |= MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset); + if (NS_WARN_IF(ret.Failed())) { + return ret; + } } if (brNode) { rv = htmlEditor->DeleteNode(brNode); // XXX In other top level if/else-if blocks, the result of DeleteNode() // is ignored. Why does only this result is respected? - NS_ENSURE_SUCCESS(rv, rv); - *aHandled = true; + if (NS_WARN_IF(NS_FAILED(rv))) { + return ret.SetResult(rv); + } + ret.MarkAsHandled(); } } - return NS_OK; + return ret; } -nsresult +EditActionResult HTMLEditRules::MoveBlock(Element& aLeftBlock, Element& aRightBlock, int32_t aLeftOffset, - int32_t aRightOffset, - bool* aHandled) + int32_t aRightOffset) { - MOZ_ASSERT(aHandled); - - *aHandled = false; - nsTArray> arrayOfNodes; // GetNodesFromPoint is the workhorse that figures out what we wnat to move. nsresult rv = GetNodesFromPoint(EditorDOMPoint(&aRightBlock, aRightOffset), EditAction::makeList, arrayOfNodes, TouchContent::yes); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } + + EditActionResult ret(NS_OK); for (uint32_t i = 0; i < arrayOfNodes.Length(); i++) { // get the node to act on if (IsBlockNode(arrayOfNodes[i])) { // For block nodes, move their contents only, then delete block. - bool handled = false; - rv = MoveContents(*arrayOfNodes[i]->AsElement(), aLeftBlock, - &aLeftOffset, &handled); - *aHandled |= handled; - NS_ENSURE_SUCCESS(rv, rv); - NS_ENSURE_STATE(mHTMLEditor); + ret |= + MoveContents(*arrayOfNodes[i]->AsElement(), aLeftBlock, &aLeftOffset); + if (NS_WARN_IF(ret.Failed())) { + return ret; + } + if (NS_WARN_IF(!mHTMLEditor)) { + return ret.SetResult(NS_ERROR_UNEXPECTED); + } rv = mHTMLEditor->DeleteNode(arrayOfNodes[i]); - *aHandled = true; + ret.MarkAsHandled(); } else { // Otherwise move the content as is, checking against the DTD. - bool handled = false; - rv = MoveNodeSmart(*arrayOfNodes[i]->AsContent(), aLeftBlock, - &aLeftOffset, &handled); - *aHandled |= handled; + ret |= + MoveNodeSmart(*arrayOfNodes[i]->AsContent(), aLeftBlock, &aLeftOffset); } } // XXX We're only checking return value of the last iteration - NS_ENSURE_SUCCESS(rv, rv); - return NS_OK; + if (NS_WARN_IF(ret.Failed())) { + return ret; + } + + return ret; } -nsresult +EditActionResult HTMLEditRules::MoveNodeSmart(nsIContent& aNode, Element& aDestElement, - int32_t* aInOutDestOffset, - bool* aHandled) + int32_t* aInOutDestOffset) { MOZ_ASSERT(aInOutDestOffset); - MOZ_ASSERT(aHandled); - *aHandled = false; + if (NS_WARN_IF(!mHTMLEditor)) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } - NS_ENSURE_STATE(mHTMLEditor); RefPtr htmlEditor(mHTMLEditor); // Check if this node can go into the destination node @@ -2928,50 +2962,52 @@ HTMLEditRules::MoveNodeSmart(nsIContent& aNode, // If it can, move it there nsresult rv = htmlEditor->MoveNode(&aNode, &aDestElement, *aInOutDestOffset); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } if (*aInOutDestOffset != -1) { (*aInOutDestOffset)++; } // XXX Should we check if the node is actually moved in this case? - *aHandled = true; + return EditActionHandled(); } else { // If it can't, move its children (if any), and then delete it. + EditActionResult ret(NS_OK); if (aNode.IsElement()) { - bool handled = false; - nsresult rv = MoveContents(*aNode.AsElement(), aDestElement, - aInOutDestOffset, &handled); - *aHandled |= handled; - NS_ENSURE_SUCCESS(rv, rv); + ret = MoveContents(*aNode.AsElement(), aDestElement, aInOutDestOffset); + if (NS_WARN_IF(ret.Failed())) { + return ret; + } } nsresult rv = htmlEditor->DeleteNode(&aNode); - NS_ENSURE_SUCCESS(rv, rv); - *aHandled = true; + if (NS_WARN_IF(NS_FAILED(rv))) { + return ret.SetResult(rv); + } + return ret.MarkAsHandled(); } - return NS_OK; } -nsresult +EditActionResult HTMLEditRules::MoveContents(Element& aElement, Element& aDestElement, - int32_t* aInOutDestOffset, - bool* aHandled) + int32_t* aInOutDestOffset) { MOZ_ASSERT(aInOutDestOffset); - MOZ_ASSERT(aHandled); - *aHandled = false; - - NS_ENSURE_TRUE(&aElement != &aDestElement, NS_ERROR_ILLEGAL_VALUE); - - while (aElement.GetFirstChild()) { - bool handled = false; - nsresult rv = MoveNodeSmart(*aElement.GetFirstChild(), aDestElement, - aInOutDestOffset, &handled); - *aHandled |= handled; - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(&aElement == &aDestElement)) { + return EditActionIgnored(NS_ERROR_ILLEGAL_VALUE); } - return NS_OK; + + EditActionResult ret(NS_OK); + while (aElement.GetFirstChild()) { + ret |= + MoveNodeSmart(*aElement.GetFirstChild(), aDestElement, aInOutDestOffset); + if (NS_WARN_IF(ret.Failed())) { + return ret; + } + } + return ret; } diff --git a/editor/libeditor/HTMLEditRules.h b/editor/libeditor/HTMLEditRules.h index 6cdfa57cf1..5525fdf249 100644 --- a/editor/libeditor/HTMLEditRules.h +++ b/editor/libeditor/HTMLEditRules.h @@ -28,6 +28,7 @@ class nsRange; namespace mozilla { +class EditActionResult; class HTMLEditor; class RulesInfo; class TextEditor; @@ -173,20 +174,16 @@ protected: * placing its children into leftblock. DTD containment rules are followed * throughout. * - * @param aCanceled returns true if the operation should do nothing anymore - * even if this doesn't join the blocks. - * NOTE: When this returns an error, nobody should refer - * the result of this. - * @param aHandled returns true if this actually handles the request. - * Note that this may return true even if this does not - * join the block. E.g., if the blocks shouldn't be - * joined or it's impossible to join them but it's not + * @return Sets canceled to true if the operation should do + * nothing anymore even if this doesn't join the blocks. + * Sets handled to true if this actually handles the + * request. Note that this may set it to true even if this + * does not join the block. E.g., if the blocks shouldn't + * be joined or it's impossible to join them but it's not * unexpected case, this returns true with this. - * NOTE: When this returns an error, nobody should refer - * the result of this. */ - nsresult TryToJoinBlocks(nsIContent& aLeftNode, nsIContent& aRightNode, - bool* aCanceled, bool* aHandled); + EditActionResult TryToJoinBlocks(nsIContent& aLeftNode, + nsIContent& aRightNode); /** * MoveBlock() moves the content from aRightBlock starting from aRightOffset @@ -194,39 +191,36 @@ protected: * between
s, or between blocks, etc. DTD containment rules are followed * throughout. * - * @param aHandled returns true if this actually joins the nodes. - * NOTE: When this returns an error, nobody should refer - * the result of this. + * @return Sets handled to true if this actually joins the nodes. + * canceled is always false. */ - nsresult MoveBlock(Element& aLeftBlock, Element& aRightBlock, - int32_t aLeftOffset, int32_t aRightOffset, - bool* aHandled); + EditActionResult MoveBlock(Element& aLeftBlock, Element& aRightBlock, + int32_t aLeftOffset, int32_t aRightOffset); /** * MoveNodeSmart() moves aNode to (aDestElement, aInOutDestOffset). * DTD containment rules are followed throughout. * * @param aOffset returns the point after inserted content. - * @param aHandled returns true if this actually moves the - * nodes. - * NOTE: When this returns an error, nobody - * should refer the result of this. + * @return Sets true to handled if this actually moves + * the nodes. + * canceled is always false. */ - nsresult MoveNodeSmart(nsIContent& aNode, Element& aDestElement, - int32_t* aInOutDestOffset, bool* aHandled); + EditActionResult MoveNodeSmart(nsIContent& aNode, Element& aDestElement, + int32_t* aInOutDestOffset); /** * MoveContents() moves the contents of aElement to (aDestElement, * aInOutDestOffset). DTD containment rules are followed throughout. * * @param aInOutDestOffset updated to point after inserted content. - * @param aHandled returns true if this actually moves the - * nodes. - * NOTE: When this returns an error, nobody - * should refer the result of this. + * @return Sets true to handled if this actually moves + * the nodes. + * canceled is always false. */ - nsresult MoveContents(Element& aElement, Element& aDestElement, - int32_t* aInOutDestOffset, bool* aHandled); + EditActionResult MoveContents(Element& aElement, Element& aDestElement, + int32_t* aInOutDestOffset); + nsresult DeleteNonTableElements(nsINode* aNode); nsresult WillMakeList(Selection* aSelection, const nsAString* aListType, From 1a100c40d74f04dbee58834be5c1f3d190d66962 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 11:55:44 -0400 Subject: [PATCH 12/13] Bug 1316302 - Part 4: Refine HTMLEditRules::TryToJoinBlocks() and HTMLEditRules::MoveNodeSmart() with early return style for making scope of EditActionResult variable smaller For now, let's make the scope of EditActionResult variable in them smaller without big change. Tag #1563 --- editor/libeditor/HTMLEditRules.cpp | 116 +++++++++++++++-------------- 1 file changed, 61 insertions(+), 55 deletions(-) diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index ef5456f62e..af4a43ab9e 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -2683,7 +2683,6 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, // offset below is where you find yourself in rightBlock when you traverse // upwards from leftBlock - EditActionResult ret(NS_OK); if (EditorUtils::IsDescendantOf(leftBlock, rightBlock, &rightOffset)) { // Tricky case. Left block is inside right block. Do ws adjustment. This // just destroys non-visible ws at boundaries we will be joining. @@ -2719,6 +2718,7 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, // Do br adjustment. nsCOMPtr brNode = CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd); + EditActionResult ret(NS_OK); if (mergeLists) { // The idea here is to take all children in rightList that are past // offset, and pull them into leftlist. @@ -2742,9 +2742,12 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { ret.MarkAsHandled(); } + return ret; + } + // Offset below is where you find yourself in leftBlock when you traverse // upwards from rightBlock - } else if (EditorUtils::IsDescendantOf(rightBlock, leftBlock, &leftOffset)) { + if (EditorUtils::IsDescendantOf(rightBlock, leftBlock, &leftOffset)) { // Tricky case. Right block is inside left block. Do ws adjustment. This // just destroys non-visible ws at boundaries we will be joining. nsresult rv = WSRunObject::ScrubBlockBoundary(htmlEditor, @@ -2779,6 +2782,7 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, // Do br adjustment. nsCOMPtr brNode = CheckForInvisibleBR(*leftBlock, BRLocation::beforeBlock, leftOffset); + EditActionResult ret(NS_OK); if (mergeLists) { // XXX Why do we ignore the result of MoveContents()? EditActionResult retMoveContents = @@ -2854,47 +2858,49 @@ HTMLEditRules::TryToJoinBlocks(nsIContent& aLeftNode, if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { ret.MarkAsHandled(); } - } else { - // Normal case. Blocks are siblings, or at least close enough. An example - // of the latter is

paragraph

  • one
  • two
  • three
. The - // first li and the p are not true siblings, but we still want to join them - // if you backspace from li into p. + return ret; + } - // Adjust whitespace at block boundaries - nsresult rv = - WSRunObject::PrepareToJoinBlocks(htmlEditor, leftBlock, rightBlock); + // Normal case. Blocks are siblings, or at least close enough. An example + // of the latter is

paragraph

  • one
  • two
  • three
. The + // first li and the p are not true siblings, but we still want to join them + // if you backspace from li into p. + + // Adjust whitespace at block boundaries + nsresult rv = + WSRunObject::PrepareToJoinBlocks(htmlEditor, leftBlock, rightBlock); + if (NS_WARN_IF(NS_FAILED(rv))) { + return EditActionIgnored(rv); + } + // Do br adjustment. + nsCOMPtr brNode = + CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd); + EditActionResult ret(NS_OK); + if (mergeLists || leftBlock->NodeInfo()->NameAtom() == + rightBlock->NodeInfo()->NameAtom()) { + // Nodes are same type. merge them. + EditorDOMPoint pt = JoinNodesSmart(*leftBlock, *rightBlock); + if (pt.node && mergeLists) { + nsCOMPtr newBlock; + ConvertListType(rightBlock, getter_AddRefs(newBlock), + existingList, nsGkAtoms::li); + } + ret.MarkAsHandled(); + } else { + // Nodes are dissimilar types. + ret |= MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset); + if (NS_WARN_IF(ret.Failed())) { + return ret; + } + } + if (brNode) { + rv = htmlEditor->DeleteNode(brNode); + // XXX In other top level if blocks, the result of DeleteNode() + // is ignored. Why does only this result is respected? if (NS_WARN_IF(NS_FAILED(rv))) { - return EditActionIgnored(rv); - } - // Do br adjustment. - nsCOMPtr brNode = - CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd); - if (mergeLists || leftBlock->NodeInfo()->NameAtom() == - rightBlock->NodeInfo()->NameAtom()) { - // Nodes are same type. merge them. - EditorDOMPoint pt = JoinNodesSmart(*leftBlock, *rightBlock); - if (pt.node && mergeLists) { - nsCOMPtr newBlock; - ConvertListType(rightBlock, getter_AddRefs(newBlock), - existingList, nsGkAtoms::li); - } - ret.MarkAsHandled(); - } else { - // Nodes are dissimilar types. - ret |= MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset); - if (NS_WARN_IF(ret.Failed())) { - return ret; - } - } - if (brNode) { - rv = htmlEditor->DeleteNode(brNode); - // XXX In other top level if/else-if blocks, the result of DeleteNode() - // is ignored. Why does only this result is respected? - if (NS_WARN_IF(NS_FAILED(rv))) { - return ret.SetResult(rv); - } - ret.MarkAsHandled(); + return ret.SetResult(rv); } + ret.MarkAsHandled(); } return ret; } @@ -2970,22 +2976,22 @@ HTMLEditRules::MoveNodeSmart(nsIContent& aNode, } // XXX Should we check if the node is actually moved in this case? return EditActionHandled(); - } else { - // If it can't, move its children (if any), and then delete it. - EditActionResult ret(NS_OK); - if (aNode.IsElement()) { - ret = MoveContents(*aNode.AsElement(), aDestElement, aInOutDestOffset); - if (NS_WARN_IF(ret.Failed())) { - return ret; - } - } - - nsresult rv = htmlEditor->DeleteNode(&aNode); - if (NS_WARN_IF(NS_FAILED(rv))) { - return ret.SetResult(rv); - } - return ret.MarkAsHandled(); } + + // If it can't, move its children (if any), and then delete it. + EditActionResult ret(NS_OK); + if (aNode.IsElement()) { + ret = MoveContents(*aNode.AsElement(), aDestElement, aInOutDestOffset); + if (NS_WARN_IF(ret.Failed())) { + return ret; + } + } + + nsresult rv = htmlEditor->DeleteNode(&aNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return ret.SetResult(rv); + } + return ret.MarkAsHandled(); } EditActionResult From 9f5067486bf676842317d0b30f7bb2c559e603ec Mon Sep 17 00:00:00 2001 From: Lootyhoof Date: Tue, 9 Jun 2020 20:01:57 +0100 Subject: [PATCH 13/13] [Pale-Moon] Issue MoonchildProductions/UXP#1578 - Add global menubar support for GTK --- application/palemoon/base/content/browser-menubar.inc | 4 ++++ application/palemoon/base/content/browser.css | 4 ++++ application/palemoon/base/content/browser.js | 6 ++++++ application/palemoon/components/places/content/places.xul | 4 ++++ 4 files changed, 18 insertions(+) diff --git a/application/palemoon/base/content/browser-menubar.inc b/application/palemoon/base/content/browser-menubar.inc index fc6bc76943..8651d07bf4 100644 --- a/application/palemoon/base/content/browser-menubar.inc +++ b/application/palemoon/base/content/browser-menubar.inc @@ -5,7 +5,11 @@ diff --git a/application/palemoon/base/content/browser.css b/application/palemoon/base/content/browser.css index 91c6478d2d..2bab69b228 100644 --- a/application/palemoon/base/content/browser.css +++ b/application/palemoon/base/content/browser.css @@ -226,6 +226,10 @@ splitmenu { #appmenu-toolbar-button > .toolbarbutton-text { display: -moz-box; } + +window[shellshowingmenubar="true"] #appmenu-toolbar-button { + display: none; +} %endif #appmenu_offlineModeRecovery:not([checked=true]) { diff --git a/application/palemoon/base/content/browser.js b/application/palemoon/base/content/browser.js index 09a0ff4de7..71e5320a75 100644 --- a/application/palemoon/base/content/browser.js +++ b/application/palemoon/base/content/browser.js @@ -4600,6 +4600,12 @@ function onViewToolbarsPopupShowing(aEvent, aInsertPoint) { toolbarNodes.push(document.getElementById("addon-bar")); for (let toolbar of toolbarNodes) { +#ifdef MOZ_WIDGET_GTK + if (toolbar.id == "toolbar-menubar" && + document.documentElement.getAttribute("shellshowingmenubar") == "true") { + continue; + } +#endif let toolbarName = toolbar.getAttribute("toolbarname"); if (toolbarName) { let menuItem = document.createElement("menuitem"); diff --git a/application/palemoon/components/places/content/places.xul b/application/palemoon/components/places/content/places.xul index 92e8a70e9c..fd7843e7fb 100644 --- a/application/palemoon/components/places/content/places.xul +++ b/application/palemoon/components/places/content/places.xul @@ -153,8 +153,12 @@ #ifdef XP_MACOSX #else +#endif