From d6baead6c8bcd90f04d62908bfaf73b369df4e6f Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 20 Dec 2019 23:08:47 +0100 Subject: Issue #1219 - Align computed DOM styles with mainstream behvior. This updates our behavior for computed DOM styling to no longer return null on elements that have no display, but return a 0-length (empty) style instead and don't throw. For this we stop looking at having a presentation for the style and just look at the document instead. This resolves #1219 --- layout/style/nsComputedDOMStyle.cpp | 67 ++++++++++++++++++++----------------- layout/style/nsComputedDOMStyle.h | 12 +++---- 2 files changed, 42 insertions(+), 37 deletions(-) (limited to 'layout/style') diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp index 65c1d698c..647f7f6dc 100644 --- a/layout/style/nsComputedDOMStyle.cpp +++ b/layout/style/nsComputedDOMStyle.cpp @@ -64,13 +64,13 @@ using namespace mozilla::dom; */ already_AddRefed -NS_NewComputedDOMStyle(dom::Element* aElement, const nsAString& aPseudoElt, - nsIPresShell* aPresShell, +NS_NewComputedDOMStyle(dom::Element* aElement, + const nsAString& aPseudoElt, + nsIDocument* aDocument, nsComputedDOMStyle::StyleType aStyleType) { RefPtr computedStyle; - computedStyle = new nsComputedDOMStyle(aElement, aPseudoElt, aPresShell, - aStyleType); + computedStyle = new nsComputedDOMStyle(aElement, aPseudoElt, aDocument, aStyleType); return computedStyle.forget(); } @@ -243,7 +243,7 @@ nsComputedStyleMap::Update() nsComputedDOMStyle::nsComputedDOMStyle(dom::Element* aElement, const nsAString& aPseudoElt, - nsIPresShell* aPresShell, + nsIDocument* aDocument, StyleType aStyleType) : mDocumentWeak(nullptr) , mOuterFrame(nullptr) @@ -254,11 +254,13 @@ nsComputedDOMStyle::nsComputedDOMStyle(dom::Element* aElement, , mExposeVisitedStyle(false) , mResolvedStyleContext(false) { - MOZ_ASSERT(aElement && aPresShell); + MOZ_ASSERT(aElement); + MOZ_ASSERT(aDocument); + // TODO(emilio, bug 548397, https://github.com/w3c/csswg-drafts/issues/2403): + // Should use aElement->OwnerDoc() instead. + mDocumentWeak = do_GetWeakReference(aDocument); - mDocumentWeak = do_GetWeakReference(aPresShell->GetDocument()); - - mContent = aElement; + mElement = aElement; if (!DOMStringIsNull(aPseudoElt) && !aPseudoElt.IsEmpty() && aPseudoElt.First() == char16_t(':')) { @@ -285,8 +287,6 @@ nsComputedDOMStyle::nsComputedDOMStyle(dom::Element* aElement, mPseudo = nullptr; } } - - MOZ_ASSERT(aPresShell->GetPresContext()); } nsComputedDOMStyle::~nsComputedDOMStyle() @@ -297,13 +297,13 @@ nsComputedDOMStyle::~nsComputedDOMStyle() NS_IMPL_CYCLE_COLLECTION_CLASS(nsComputedDOMStyle) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsComputedDOMStyle) - tmp->ClearStyleContext(); // remove observer before clearing mContent - NS_IMPL_CYCLE_COLLECTION_UNLINK(mContent) + tmp->ClearStyleContext(); // remove observer before clearing mElement + NS_IMPL_CYCLE_COLLECTION_UNLINK(mElement) NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsComputedDOMStyle) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mContent) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mElement) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(nsComputedDOMStyle) @@ -366,8 +366,6 @@ nsComputedDOMStyle::SetCssText(const nsAString& aCssText) NS_IMETHODIMP nsComputedDOMStyle::GetLength(uint32_t* aLength) { - NS_PRECONDITION(aLength, "Null aLength! Prepare to die!"); - uint32_t length = GetComputedStyleMap()->Length(); // Make sure we have up to date style so that we can include custom @@ -375,6 +373,8 @@ nsComputedDOMStyle::GetLength(uint32_t* aLength) UpdateCurrentStyleSources(false); if (mStyleContext) { length += StyleVariables()->mVariables.Count(); + } else { + length = 0; } *aLength = length; @@ -616,7 +616,7 @@ nsComputedDOMStyle::ClearStyleContext() { if (mResolvedStyleContext) { mResolvedStyleContext = false; - mContent->RemoveMutationObserver(this); + mElement->RemoveMutationObserver(this); } mStyleContext = nullptr; } @@ -626,7 +626,7 @@ nsComputedDOMStyle::SetResolvedStyleContext(RefPtr&& aContext) { if (!mResolvedStyleContext) { mResolvedStyleContext = true; - mContent->AddMutationObserver(this); + mElement->AddMutationObserver(this); } mStyleContext = aContext; } @@ -651,7 +651,7 @@ nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) // Flush _before_ getting the presshell, since that could create a new // presshell. Also note that we want to flush the style on the document - // we're computing style in, not on the document mContent is in -- the two + // we're computing style in, not on the document mElement is in -- the two // may be different. document->FlushPendingNotifications( aNeedsLayoutFlush ? Flush_Layout : Flush_Style); @@ -659,7 +659,7 @@ nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) mFlushedPendingReflows = aNeedsLayoutFlush; #endif - nsCOMPtr presShellForContent = GetPresShellForContent(mContent); + nsCOMPtr presShellForContent = GetPresShellForContent(mElement); if (presShellForContent && presShellForContent != document->GetShell()) { presShellForContent->FlushPendingNotifications(Flush_Style); } @@ -674,7 +674,11 @@ nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) mPresShell->GetPresContext()->GetRestyleGeneration(); if (mStyleContext) { - if (mStyleContextGeneration == currentGeneration) { + // We can't rely on the undisplayed restyle generation if mElement is + // out-of-document, since that generation is not incremented for DOM changes + // on out-of-document elements. + if (mStyleContextGeneration == currentGeneration && + mElement->IsInComposedDoc()) { // Our cached style context is still valid. return; } @@ -683,12 +687,12 @@ nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) mStyleContext = nullptr; } - // XXX the !mContent->IsHTMLElement(nsGkAtoms::area) + // XXX the !mElement->IsHTMLElement(nsGkAtoms::area) // check is needed due to bug 135040 (to avoid using // mPrimaryFrame). Remove it once that's fixed. if (!mPseudo && mStyleType == eAll && - !mContent->IsHTMLElement(nsGkAtoms::area)) { - mOuterFrame = mContent->GetPrimaryFrame(); + !mElement->IsHTMLElement(nsGkAtoms::area)) { + mOuterFrame = mElement->GetPrimaryFrame(); mInnerFrame = mOuterFrame; if (mOuterFrame) { nsIAtom* type = mOuterFrame->GetType(); @@ -731,7 +735,7 @@ nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) // Need to resolve a style context RefPtr resolvedStyleContext = nsComputedDOMStyle::GetStyleContextForElementNoFlush( - mContent->AsElement(), + mElement, mPseudo, presShellForContent ? presShellForContent.get() : mPresShell, mStyleType); @@ -824,7 +828,6 @@ nsComputedDOMStyle::GetPropertyCSSValue(const nsAString& aPropertyName, ErrorRes UpdateCurrentStyleSources(needsLayoutFlush); if (!mStyleContext) { - aRv.Throw(NS_ERROR_NOT_AVAILABLE); return nullptr; } @@ -2861,7 +2864,7 @@ nsComputedDOMStyle::DoGetGridTemplateColumns() nsGridContainerFrame* gridFrame = nsGridContainerFrame::GetGridFrameWithComputedInfo( - mContent->GetPrimaryFrame()); + mElement->GetPrimaryFrame()); if (gridFrame) { info = gridFrame->GetComputedTemplateColumns(); @@ -2877,7 +2880,7 @@ nsComputedDOMStyle::DoGetGridTemplateRows() nsGridContainerFrame* gridFrame = nsGridContainerFrame::GetGridFrameWithComputedInfo( - mContent->GetPrimaryFrame()); + mElement->GetPrimaryFrame()); if (gridFrame) { info = gridFrame->GetComputedTemplateRows(); @@ -5214,8 +5217,10 @@ nsComputedDOMStyle::GetLineHeightCoord(nscoord& aCoord) // lie about font size inflation since we lie about font size (since // the inflation only applies to text) - aCoord = ReflowInput::CalcLineHeight(mContent, mStyleContext, - blockHeight, 1.0f); + aCoord = ReflowInput::CalcLineHeight(mElement, + mStyleContext, + blockHeight, + 1.0f); // CalcLineHeight uses font->mFont.size, but we want to use // font->mSize as the font size. Adjust for that. Also adjust for @@ -6642,7 +6647,7 @@ nsComputedDOMStyle::DoGetCustomProperty(const nsAString& aPropertyName) void nsComputedDOMStyle::ParentChainChanged(nsIContent* aContent) { - NS_ASSERTION(mContent == aContent, "didn't we register mContent?"); + NS_ASSERTION(mElement == aContent, "didn't we register mElement?"); NS_ASSERTION(mResolvedStyleContext, "should have only registered an observer when " "mResolvedStyleContext is true"); diff --git a/layout/style/nsComputedDOMStyle.h b/layout/style/nsComputedDOMStyle.h index 77df71ec8..7fbf49afe 100644 --- a/layout/style/nsComputedDOMStyle.h +++ b/layout/style/nsComputedDOMStyle.h @@ -76,12 +76,12 @@ public: nsComputedDOMStyle(mozilla::dom::Element* aElement, const nsAString& aPseudoElt, - nsIPresShell* aPresShell, + nsIDocument* aDocument, StyleType aStyleType); - virtual nsINode *GetParentObject() override + nsINode *GetParentObject() override { - return mContent; + return mElement; } static already_AddRefed @@ -667,9 +667,9 @@ private: // We don't really have a good immutable representation of "presentation". // Given the way GetComputedStyle is currently used, we should just grab the - // 0th presshell, if any, from the document. + // presshell, if any, from the document. nsWeakPtr mDocumentWeak; - nsCOMPtr mContent; + RefPtr mElement; /** * Strong reference to the style context we access data from. This can be @@ -735,7 +735,7 @@ private: already_AddRefed NS_NewComputedDOMStyle(mozilla::dom::Element* aElement, const nsAString& aPseudoElt, - nsIPresShell* aPresShell, + nsIDocument* aDocument, nsComputedDOMStyle::StyleType aStyleType = nsComputedDOMStyle::eAll); -- cgit v1.2.3