From 91d2c4c05a3fa243c7a4a3c12394bde51e1072a2 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 31 Jul 2020 20:27:48 -0400 Subject: [PATCH 01/16] Use an alt script to properly determine the OSX SDK version --- media/libyuv/build/mac/find_sdk_uxp.py | 12 +++++++ media/webrtc/trunk/build/common.gypi | 2 +- media/webrtc/trunk/build/mac/find_sdk_uxp.py | 35 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 media/libyuv/build/mac/find_sdk_uxp.py create mode 100644 media/webrtc/trunk/build/mac/find_sdk_uxp.py diff --git a/media/libyuv/build/mac/find_sdk_uxp.py b/media/libyuv/build/mac/find_sdk_uxp.py new file mode 100644 index 0000000000..b81dba9470 --- /dev/null +++ b/media/libyuv/build/mac/find_sdk_uxp.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python +# 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/. +import sys +import subprocess + +def main(): + return subprocess.call([sys.executable, "../webrtc/trunk/build/mac/find_sdk_uxp.py"] + sys.argv[1:]) + +if __name__ == '__main__': + sys.exit(main()) diff --git a/media/webrtc/trunk/build/common.gypi b/media/webrtc/trunk/build/common.gypi index 68003ba06f..47ad77de48 100644 --- a/media/webrtc/trunk/build/common.gypi +++ b/media/webrtc/trunk/build/common.gypi @@ -1176,7 +1176,7 @@ # Enable Keystone auto-update support. 'mac_keystone%': 1, }, { # else: branding!="Chrome" or buildtype!="Official" - 'mac_sdk%': ' Date: Fri, 31 Jul 2020 21:08:26 -0400 Subject: [PATCH 02/16] Add license header to media/webrtc/trunk/build/mac/find_sdk_uxp.py --- media/webrtc/trunk/build/mac/find_sdk_uxp.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/media/webrtc/trunk/build/mac/find_sdk_uxp.py b/media/webrtc/trunk/build/mac/find_sdk_uxp.py index cc4b440d20..3c419f1e45 100644 --- a/media/webrtc/trunk/build/mac/find_sdk_uxp.py +++ b/media/webrtc/trunk/build/mac/find_sdk_uxp.py @@ -1,4 +1,7 @@ #!/usr/bin/env python +# 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/. from __future__ import print_function import os From 3c02d3fc0d8068a5b29ce8ad52fc07beb98cb3a0 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 31 Jul 2020 13:01:18 -0700 Subject: [PATCH 03/16] Issue #1619 - Convert Intrinsic Ratio to Float https://bugzilla.mozilla.org/show_bug.cgi?id=1547792 Aspect Ratio handling simplified by using floating point integers: - Multiplication of value (or inverse value) to a known side for Scaling - No unequal equal values such as "4/3" vs "8/6" vs "20/15" - Truly "Empty" aspect ratios, even if one dimension is not 0 --- image/ClippedImage.cpp | 4 +- image/ClippedImage.h | 2 +- image/DynamicImage.cpp | 6 +- image/ImageWrapper.cpp | 4 +- image/OrientedImage.cpp | 4 +- image/OrientedImage.h | 2 +- image/RasterImage.cpp | 5 +- image/VectorImage.cpp | 2 +- image/imgIContainer.idl | 4 +- layout/base/nsCSSRendering.cpp | 78 ++++++++++++++---------- layout/base/nsCSSRendering.h | 22 +++---- layout/base/nsLayoutUtils.cpp | 64 ++++++++----------- layout/base/nsLayoutUtils.h | 6 +- layout/generic/AspectRatio.h | 81 +++++++++++++++++++++++++ layout/generic/moz.build | 1 + layout/generic/nsFlexContainerFrame.cpp | 31 +++++----- layout/generic/nsFrame.cpp | 77 +++++++++++++---------- layout/generic/nsFrame.h | 4 +- layout/generic/nsHTMLCanvasFrame.cpp | 30 +++------ layout/generic/nsHTMLCanvasFrame.h | 2 +- layout/generic/nsIFrame.h | 14 ++--- layout/generic/nsImageFrame.cpp | 13 ++-- layout/generic/nsImageFrame.h | 4 +- layout/generic/nsLineLayout.cpp | 2 +- layout/generic/nsSubDocumentFrame.cpp | 4 +- layout/generic/nsSubDocumentFrame.h | 2 +- layout/generic/nsVideoFrame.cpp | 18 +++--- layout/generic/nsVideoFrame.h | 2 +- layout/style/nsStyleStruct.cpp | 4 +- layout/svg/nsSVGOuterSVGFrame.cpp | 44 +++++++------- layout/svg/nsSVGOuterSVGFrame.h | 2 +- layout/xul/nsImageBoxFrame.cpp | 5 +- 32 files changed, 314 insertions(+), 229 deletions(-) create mode 100644 layout/generic/AspectRatio.h diff --git a/image/ClippedImage.cpp b/image/ClippedImage.cpp index 5394718512..6372f59505 100644 --- a/image/ClippedImage.cpp +++ b/image/ClippedImage.cpp @@ -242,13 +242,13 @@ ClippedImage::GetIntrinsicSize(nsSize* aSize) } NS_IMETHODIMP -ClippedImage::GetIntrinsicRatio(nsSize* aRatio) +ClippedImage::GetIntrinsicRatio(AspectRatio* aRatio) { if (!ShouldClip()) { return InnerImage()->GetIntrinsicRatio(aRatio); } - *aRatio = nsSize(mClip.width, mClip.height); + *aRatio = AspectRatio::FromSize(mClip.width, mClip.height); return NS_OK; } diff --git a/image/ClippedImage.h b/image/ClippedImage.h index 140cc19090..65963b7562 100644 --- a/image/ClippedImage.h +++ b/image/ClippedImage.h @@ -35,7 +35,7 @@ public: NS_IMETHOD GetWidth(int32_t* aWidth) override; NS_IMETHOD GetHeight(int32_t* aHeight) override; NS_IMETHOD GetIntrinsicSize(nsSize* aSize) override; - NS_IMETHOD GetIntrinsicRatio(nsSize* aRatio) override; + NS_IMETHOD GetIntrinsicRatio(AspectRatio* aRatio) override; NS_IMETHOD_(already_AddRefed) GetFrame(uint32_t aWhichFrame, uint32_t aFlags) override; NS_IMETHOD_(already_AddRefed) diff --git a/image/DynamicImage.cpp b/image/DynamicImage.cpp index 670595aecb..aeb1fcf78e 100644 --- a/image/DynamicImage.cpp +++ b/image/DynamicImage.cpp @@ -136,10 +136,10 @@ DynamicImage::GetIntrinsicSize(nsSize* aSize) } NS_IMETHODIMP -DynamicImage::GetIntrinsicRatio(nsSize* aSize) +DynamicImage::GetIntrinsicRatio(AspectRatio* aRatio) { - IntSize intSize(mDrawable->Size()); - *aSize = nsSize(intSize.width, intSize.height); + auto size = mDrawable->Size(); + *aRatio = AspectRatio::FromSize(size); return NS_OK; } diff --git a/image/ImageWrapper.cpp b/image/ImageWrapper.cpp index f7068dfc58..5ed6c78179 100644 --- a/image/ImageWrapper.cpp +++ b/image/ImageWrapper.cpp @@ -146,9 +146,9 @@ ImageWrapper::GetIntrinsicSize(nsSize* aSize) } NS_IMETHODIMP -ImageWrapper::GetIntrinsicRatio(nsSize* aSize) +ImageWrapper::GetIntrinsicRatio(AspectRatio* aRatio) { - return mInnerImage->GetIntrinsicRatio(aSize); + return mInnerImage->GetIntrinsicRatio(aRatio); } NS_IMETHODIMP_(Orientation) diff --git a/image/OrientedImage.cpp b/image/OrientedImage.cpp index 9489ceafd0..0b6560df1d 100644 --- a/image/OrientedImage.cpp +++ b/image/OrientedImage.cpp @@ -59,12 +59,12 @@ OrientedImage::GetIntrinsicSize(nsSize* aSize) } NS_IMETHODIMP -OrientedImage::GetIntrinsicRatio(nsSize* aRatio) +OrientedImage::GetIntrinsicRatio(AspectRatio* aRatio) { nsresult rv = InnerImage()->GetIntrinsicRatio(aRatio); if (mOrientation.SwapsWidthAndHeight()) { - swap(aRatio->width, aRatio->height); + *aRatio = aRatio->Inverted(); } return rv; diff --git a/image/OrientedImage.h b/image/OrientedImage.h index 604ab3d4f5..b77571f4a3 100644 --- a/image/OrientedImage.h +++ b/image/OrientedImage.h @@ -31,7 +31,7 @@ public: NS_IMETHOD GetWidth(int32_t* aWidth) override; NS_IMETHOD GetHeight(int32_t* aHeight) override; NS_IMETHOD GetIntrinsicSize(nsSize* aSize) override; - NS_IMETHOD GetIntrinsicRatio(nsSize* aRatio) override; + NS_IMETHOD GetIntrinsicRatio(AspectRatio* aRatio) override; NS_IMETHOD_(already_AddRefed) GetFrame(uint32_t aWhichFrame, uint32_t aFlags) override; NS_IMETHOD_(already_AddRefed) diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index b67ec3bef4..5725aee3a4 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -240,13 +240,14 @@ RasterImage::GetIntrinsicSize(nsSize* aSize) //****************************************************************************** NS_IMETHODIMP -RasterImage::GetIntrinsicRatio(nsSize* aRatio) +RasterImage::GetIntrinsicRatio(AspectRatio* aRatio) { if (mError) { return NS_ERROR_FAILURE; } - *aRatio = nsSize(mSize.width, mSize.height); + *aRatio = AspectRatio::FromSize(mSize); + return NS_OK; } diff --git a/image/VectorImage.cpp b/image/VectorImage.cpp index 3028c1c2ec..3fcb296f5f 100644 --- a/image/VectorImage.cpp +++ b/image/VectorImage.cpp @@ -628,7 +628,7 @@ VectorImage::GetIntrinsicSize(nsSize* aSize) //****************************************************************************** NS_IMETHODIMP -VectorImage::GetIntrinsicRatio(nsSize* aRatio) +VectorImage::GetIntrinsicRatio(AspectRatio* aRatio) { if (mError || !mIsFullyLoaded) { return NS_ERROR_FAILURE; diff --git a/image/imgIContainer.idl b/image/imgIContainer.idl index 20c949037f..ba1c494af6 100644 --- a/image/imgIContainer.idl +++ b/image/imgIContainer.idl @@ -12,6 +12,7 @@ #include "gfxMatrix.h" #include "gfxRect.h" #include "mozilla/gfx/2D.h" +#include "mozilla/AspectRatio.h" #include "mozilla/Maybe.h" #include "mozilla/RefPtr.h" #include "nsRect.h" @@ -55,6 +56,7 @@ native SamplingFilter(mozilla::gfx::SamplingFilter); native nsIntRectByVal(nsIntRect); [ref] native nsIntSize(nsIntSize); native nsSize(nsSize); +native AspectRatio(mozilla::AspectRatio); [ptr] native nsIFrame(nsIFrame); native TempRefImageContainer(already_AddRefed); [ref] native ImageRegion(mozilla::image::ImageRegion); @@ -101,7 +103,7 @@ interface imgIContainer : nsISupports * The (dimensionless) intrinsic ratio of this image. In the case of any * error, an exception will be thrown. */ - [noscript] readonly attribute nsSize intrinsicRatio; + [noscript] readonly attribute AspectRatio intrinsicRatio; /** * Given a size at which this image will be displayed, and the drawing diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp index 9a827546fb..e6e3b5f6fe 100644 --- a/layout/base/nsCSSRendering.cpp +++ b/layout/base/nsCSSRendering.cpp @@ -3546,10 +3546,10 @@ ComputeDrawnSizeForBackground(const CSSSizeOrRatio& aIntrinsicSize, imageSize.width = ComputeRoundedSize(imageSize.width, aBgPositioningArea.width); if (!isRepeatRoundInBothDimensions && aLayerSize.mHeightType == nsStyleImageLayers::Size::DimensionType::eAuto) { - // Restore intrinsic rato - if (aIntrinsicSize.mRatio.width) { - float scale = float(aIntrinsicSize.mRatio.height) / aIntrinsicSize.mRatio.width; - imageSize.height = NSCoordSaturatingNonnegativeMultiply(imageSize.width, scale); + // Restore intrinsic ratio + if (aIntrinsicSize.mRatio) { + imageSize.height = + aIntrinsicSize.mRatio.Inverted().ApplyTo(imageSize.width); } } } @@ -3560,10 +3560,9 @@ ComputeDrawnSizeForBackground(const CSSSizeOrRatio& aIntrinsicSize, imageSize.height = ComputeRoundedSize(imageSize.height, aBgPositioningArea.height); if (!isRepeatRoundInBothDimensions && aLayerSize.mWidthType == nsStyleImageLayers::Size::DimensionType::eAuto) { - // Restore intrinsic rato - if (aIntrinsicSize.mRatio.height) { - float scale = float(aIntrinsicSize.mRatio.width) / aIntrinsicSize.mRatio.height; - imageSize.width = NSCoordSaturatingNonnegativeMultiply(imageSize.height, scale); + // Restore intrinsic ratio + if (aIntrinsicSize.mRatio) { + imageSize.width = aIntrinsicSize.mRatio.ApplyTo(imageSize.height); } } } @@ -5264,17 +5263,11 @@ CSSSizeOrRatio::ComputeConcreteSize() const return nsSize(mWidth, mHeight); } if (mHasWidth) { - nscoord height = NSCoordSaturatingNonnegativeMultiply( - mWidth, - double(mRatio.height) / mRatio.width); - return nsSize(mWidth, height); + return nsSize(mWidth, mRatio.Inverted().ApplyTo(mWidth)); } MOZ_ASSERT(mHasHeight); - nscoord width = NSCoordSaturatingNonnegativeMultiply( - mHeight, - double(mRatio.width) / mRatio.height); - return nsSize(width, mHeight); + return nsSize(mRatio.ApplyTo(mHeight), mHeight); } CSSSizeOrRatio @@ -5379,9 +5372,7 @@ nsImageRenderer::ComputeConcreteSize(const CSSSizeOrRatio& aSpecifiedSize, if (aSpecifiedSize.mHasWidth) { nscoord height; if (aIntrinsicSize.HasRatio()) { - height = NSCoordSaturatingNonnegativeMultiply( - aSpecifiedSize.mWidth, - double(aIntrinsicSize.mRatio.height) / aIntrinsicSize.mRatio.width); + height = aIntrinsicSize.mRatio.Inverted().ApplyTo(aSpecifiedSize.mWidth); } else if (aIntrinsicSize.mHasHeight) { height = aIntrinsicSize.mHeight; } else { @@ -5393,9 +5384,7 @@ nsImageRenderer::ComputeConcreteSize(const CSSSizeOrRatio& aSpecifiedSize, MOZ_ASSERT(aSpecifiedSize.mHasHeight); nscoord width; if (aIntrinsicSize.HasRatio()) { - width = NSCoordSaturatingNonnegativeMultiply( - aSpecifiedSize.mHeight, - double(aIntrinsicSize.mRatio.width) / aIntrinsicSize.mRatio.height); + width = aIntrinsicSize.mRatio.ApplyTo(aSpecifiedSize.mHeight); } else if (aIntrinsicSize.mHasWidth) { width = aIntrinsicSize.mWidth; } else { @@ -5406,32 +5395,57 @@ nsImageRenderer::ComputeConcreteSize(const CSSSizeOrRatio& aSpecifiedSize, /* static */ nsSize nsImageRenderer::ComputeConstrainedSize(const nsSize& aConstrainingSize, - const nsSize& aIntrinsicRatio, + const AspectRatio& aIntrinsicRatio, FitType aFitType) { - if (aIntrinsicRatio.width <= 0 && aIntrinsicRatio.height <= 0) { + if (!aIntrinsicRatio) { return aConstrainingSize; } - float scaleX = double(aConstrainingSize.width) / aIntrinsicRatio.width; - float scaleY = double(aConstrainingSize.height) / aIntrinsicRatio.height; + // Suppose we're doing a "contain" fit. If the image's aspect ratio has a + // "fatter" shape than the constraint area, then we need to use the + // constraint area's full width, and we need to use the aspect ratio to + // produce a height. ON the other hand, if the aspect ratio is "skinnier", we + // use the constraint area's full height, and we use the aspect ratio to + // produce a width. (If instead we're doing a "cover" fit, then it can easily + // be seen that we should do precisely the opposite.) + // + // This is equivalent to the more descriptive alternative: + // + // AspectRatio::FromSize(aConstrainingSize) < aIntrinsicRatio + // + // But gracefully handling the case where one of the two dimensions from + // aConstrainingSize is zero. This is easy to prove since: + // + // aConstrainingSize.width / aConstrainingSize.height < aIntrinsicRatio + // + // Is trivially equivalent to: + // + // aIntrinsicRatio.width < aIntrinsicRatio * aConstrainingSize.height + // + // For the cases where height is not zero. + // + // We use float math here to avoid losing precision for very lareg backgrounds + // since we use saturating nscoord math otherwise. + const float constraintWidth = float(aConstrainingSize.width); + const float hypotheticalWidth = + aIntrinsicRatio.ApplyToFloat(aConstrainingSize.height); + nsSize size; - if ((aFitType == CONTAIN) == (scaleX < scaleY)) { + if ((aFitType == CONTAIN) == (constraintWidth < hypotheticalWidth)) { size.width = aConstrainingSize.width; - size.height = NSCoordSaturatingNonnegativeMultiply( - aIntrinsicRatio.height, scaleX); + size.height = aIntrinsicRatio.Inverted().ApplyTo(aConstrainingSize.width); // If we're reducing the size by less than one css pixel, then just use the // constraining size. if (aFitType == CONTAIN && aConstrainingSize.height - size.height < nsPresContext::AppUnitsPerCSSPixel()) { size.height = aConstrainingSize.height; } } else { - size.width = NSCoordSaturatingNonnegativeMultiply( - aIntrinsicRatio.width, scaleY); + size.height = aConstrainingSize.height; + size.width = aIntrinsicRatio.ApplyTo(aConstrainingSize.height); if (aFitType == CONTAIN && aConstrainingSize.width - size.width < nsPresContext::AppUnitsPerCSSPixel()) { size.width = aConstrainingSize.width; } - size.height = aConstrainingSize.height; } return size; } diff --git a/layout/base/nsCSSRendering.h b/layout/base/nsCSSRendering.h index 791e9656ed..8ebeb75378 100644 --- a/layout/base/nsCSSRendering.h +++ b/layout/base/nsCSSRendering.h @@ -17,6 +17,7 @@ #include "nsLayoutUtils.h" #include "nsStyleStruct.h" #include "nsIFrame.h" +#include "mozilla/AspectRatio.h" class gfxDrawable; class nsStyleContext; @@ -41,8 +42,7 @@ class ImageContainer; struct CSSSizeOrRatio { CSSSizeOrRatio() - : mRatio(0, 0) - , mHasWidth(false) + : mHasWidth(false) , mHasHeight(false) {} bool CanComputeConcreteSize() const @@ -50,12 +50,12 @@ struct CSSSizeOrRatio return mHasWidth + mHasHeight + HasRatio() >= 2; } bool IsConcrete() const { return mHasWidth && mHasHeight; } - bool HasRatio() const { return mRatio.width > 0 && mRatio.height > 0; } + bool HasRatio() const { return !!mRatio; } bool IsEmpty() const { return (mHasWidth && mWidth <= 0) || (mHasHeight && mHeight <= 0) || - mRatio.width <= 0 || mRatio.height <= 0; + !mRatio; } // CanComputeConcreteSize must return true when ComputeConcreteSize is @@ -67,7 +67,7 @@ struct CSSSizeOrRatio mWidth = aWidth; mHasWidth = true; if (mHasHeight) { - mRatio = nsSize(mWidth, mHeight); + mRatio = AspectRatio::FromSize(mWidth, mHeight); } } void SetHeight(nscoord aHeight) @@ -75,7 +75,7 @@ struct CSSSizeOrRatio mHeight = aHeight; mHasHeight = true; if (mHasWidth) { - mRatio = nsSize(mWidth, mHeight); + mRatio = AspectRatio::FromSize(mWidth, mHeight); } } void SetSize(const nsSize& aSize) @@ -84,16 +84,16 @@ struct CSSSizeOrRatio mHeight = aSize.height; mHasWidth = true; mHasHeight = true; - mRatio = aSize; + mRatio = AspectRatio::FromSize(mWidth, mHeight); } - void SetRatio(const nsSize& aRatio) + void SetRatio(const AspectRatio& aRatio) { MOZ_ASSERT(!mHasWidth || !mHasHeight, "Probably shouldn't be setting a ratio if we have a concrete size"); mRatio = aRatio; } - nsSize mRatio; + AspectRatio mRatio; nscoord mWidth; nscoord mHeight; bool mHasWidth; @@ -184,11 +184,9 @@ public: /** * Compute the size of the rendered image using either the 'cover' or * 'contain' constraints (aFitType). - * aIntrinsicRatio may be an invalid ratio, that is one or both of its - * dimensions can be less than or equal to zero. */ static nsSize ComputeConstrainedSize(const nsSize& aConstrainingSize, - const nsSize& aIntrinsicRatio, + const mozilla::AspectRatio& aIntrinsicRatio, FitType aFitType); /** * Compute the size of the rendered image (the concrete size) where no cover/ diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index fdcdcac785..74dbd63e67 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -3966,7 +3966,7 @@ nsLayoutUtils::GetTextShadowRectsUnion(const nsRect& aTextAndDecorationsRect, enum ObjectDimensionType { eWidth, eHeight }; static nscoord ComputeMissingDimension(const nsSize& aDefaultObjectSize, - const nsSize& aIntrinsicRatio, + const AspectRatio& aIntrinsicRatio, const Maybe& aSpecifiedWidth, const Maybe& aSpecifiedHeight, ObjectDimensionType aDimensionToCompute) @@ -3977,21 +3977,15 @@ ComputeMissingDimension(const nsSize& aDefaultObjectSize, // 1. "If the object has an intrinsic aspect ratio, the missing dimension of // the concrete object size is calculated using the intrinsic aspect // ratio and the present dimension." - if (aIntrinsicRatio.width > 0 && aIntrinsicRatio.height > 0) { + if (aIntrinsicRatio) { // Fill in the missing dimension using the intrinsic aspect ratio. - nscoord knownDimensionSize; - float ratio; if (aDimensionToCompute == eWidth) { - knownDimensionSize = *aSpecifiedHeight; - ratio = aIntrinsicRatio.width / aIntrinsicRatio.height; - } else { - knownDimensionSize = *aSpecifiedWidth; - ratio = aIntrinsicRatio.height / aIntrinsicRatio.width; + return aIntrinsicRatio.ApplyTo(*aSpecifiedHeight); } - return NSCoordSaturatingNonnegativeMultiply(knownDimensionSize, ratio); + return aIntrinsicRatio.Inverted().ApplyTo(*aSpecifiedWidth); } - // 2. "Otherwise, if the missing dimension is present in the object’s + // 2. "Otherwise, if the missing dimension is present in the object's // intrinsic dimensions, [...]" // NOTE: *Skipping* this case, because we already know it's not true -- we're // in this function because the missing dimension is *not* present in @@ -4029,7 +4023,7 @@ ComputeMissingDimension(const nsSize& aDefaultObjectSize, static Maybe MaybeComputeObjectFitNoneSize(const nsSize& aDefaultObjectSize, const IntrinsicSize& aIntrinsicSize, - const nsSize& aIntrinsicRatio) + const AspectRatio& aIntrinsicRatio) { // "If the object has an intrinsic height or width, its size is resolved as // if its intrinsic dimensions were given as the specified size." @@ -4076,15 +4070,13 @@ MaybeComputeObjectFitNoneSize(const nsSize& aDefaultObjectSize, static nsSize ComputeConcreteObjectSize(const nsSize& aConstraintSize, const IntrinsicSize& aIntrinsicSize, - const nsSize& aIntrinsicRatio, + const AspectRatio& aIntrinsicRatio, uint8_t aObjectFit) { // Handle default behavior (filling the container) w/ fast early return. // (Also: if there's no valid intrinsic ratio, then we have the "fill" // behavior & just use the constraint size.) - if (MOZ_LIKELY(aObjectFit == NS_STYLE_OBJECT_FIT_FILL) || - aIntrinsicRatio.width == 0 || - aIntrinsicRatio.height == 0) { + if (MOZ_LIKELY(aObjectFit == NS_STYLE_OBJECT_FIT_FILL) || !aIntrinsicRatio) { return aConstraintSize; } @@ -4171,7 +4163,7 @@ HasInitialObjectFitAndPosition(const nsStylePosition* aStylePos) /* static */ nsRect nsLayoutUtils::ComputeObjectDestRect(const nsRect& aConstraintRect, const IntrinsicSize& aIntrinsicSize, - const nsSize& aIntrinsicRatio, + const AspectRatio& aIntrinsicRatio, const nsStylePosition* aStylePos, nsPoint* aAnchorPoint) { @@ -5092,10 +5084,12 @@ nsLayoutUtils::IntrinsicForAxis(PhysicalAxis aAxis, styleMinBSize.GetCoordValue() == 0)) || styleMaxBSize.GetUnit() != eStyleUnit_None) { - nsSize ratio(aFrame->GetIntrinsicRatio()); - nscoord ratioISize = (horizontalAxis ? ratio.width : ratio.height); - nscoord ratioBSize = (horizontalAxis ? ratio.height : ratio.width); - if (ratioBSize != 0) { + AspectRatio ratio = aFrame->GetIntrinsicRatio(); + if (ratio) { + // Convert 'ratio' if necessary, so that it's storing ISize/BSize: + if (!horizontalAxis) { + ratio = ratio.Inverted(); + } AddStateBitToAncestors(aFrame, NS_FRAME_DESCENDANT_INTRINSIC_ISIZE_DEPENDS_ON_BSIZE); @@ -5111,14 +5105,14 @@ nsLayoutUtils::IntrinsicForAxis(PhysicalAxis aAxis, (aPercentageBasis.isNothing() && GetPercentBSize(styleBSize, aFrame, horizontalAxis, h))) { h = std::max(0, h - bSizeTakenByBoxSizing); - result = NSCoordMulDiv(h, ratioISize, ratioBSize); + result = ratio.ApplyTo(h); } if (GetDefiniteSize(styleMaxBSize, aFrame, !isInlineAxis, aPercentageBasis, &h) || (aPercentageBasis.isNothing() && GetPercentBSize(styleMaxBSize, aFrame, horizontalAxis, h))) { h = std::max(0, h - bSizeTakenByBoxSizing); - nscoord maxISize = NSCoordMulDiv(h, ratioISize, ratioBSize); + nscoord maxISize = ratio.ApplyTo(h); if (maxISize < result) { result = maxISize; } @@ -5131,7 +5125,7 @@ nsLayoutUtils::IntrinsicForAxis(PhysicalAxis aAxis, (aPercentageBasis.isNothing() && GetPercentBSize(styleMinBSize, aFrame, horizontalAxis, h))) { h = std::max(0, h - bSizeTakenByBoxSizing); - nscoord minISize = NSCoordMulDiv(h, ratioISize, ratioBSize); + nscoord minISize = ratio.ApplyTo(h); if (minISize > result) { result = minISize; } @@ -6594,12 +6588,12 @@ nsLayoutUtils::DrawSingleImage(gfxContext& aContext, /* static */ void nsLayoutUtils::ComputeSizeForDrawing(imgIContainer *aImage, CSSIntSize& aImageSize, /*outparam*/ - nsSize& aIntrinsicRatio, /*outparam*/ + AspectRatio& aIntrinsicRatio, /*outparam*/ bool& aGotWidth, /*outparam*/ bool& aGotHeight /*outparam*/) { aGotWidth = NS_SUCCEEDED(aImage->GetWidth(&aImageSize.width)); - aGotHeight = NS_SUCCEEDED(aImage->GetHeight(&aImageSize.height)); + aGotHeight = NS_SUCCEEDED(aImage->GetHeight(&aImageSize.height)); bool gotRatio = NS_SUCCEEDED(aImage->GetIntrinsicRatio(&aIntrinsicRatio)); if (!(aGotWidth && aGotHeight) && !gotRatio) { @@ -6607,7 +6601,7 @@ nsLayoutUtils::ComputeSizeForDrawing(imgIContainer *aImage, // decoded) and should return zero size. aGotWidth = aGotHeight = true; aImageSize = CSSIntSize(0, 0); - aIntrinsicRatio = nsSize(0, 0); + aIntrinsicRatio = AspectRatio(); } } @@ -6616,7 +6610,7 @@ nsLayoutUtils::ComputeSizeForDrawingWithFallback(imgIContainer* aImage, const nsSize& aFallbackSize) { CSSIntSize imageSize; - nsSize imageRatio; + AspectRatio imageRatio; bool gotHeight, gotWidth; ComputeSizeForDrawing(aImage, imageSize, imageRatio, gotWidth, gotHeight); @@ -6624,19 +6618,13 @@ nsLayoutUtils::ComputeSizeForDrawingWithFallback(imgIContainer* aImage, // intrinsic ratio of the image. if (gotWidth != gotHeight) { if (!gotWidth) { - if (imageRatio.height != 0) { - imageSize.width = - NSCoordSaturatingNonnegativeMultiply(imageSize.height, - float(imageRatio.width) / - float(imageRatio.height)); + if (imageRatio) { + imageSize.width = imageRatio.ApplyTo(imageSize.height); gotWidth = true; } } else { - if (imageRatio.width != 0) { - imageSize.height = - NSCoordSaturatingNonnegativeMultiply(imageSize.width, - float(imageRatio.height) / - float(imageRatio.width)); + if (imageRatio) { + imageSize.height = imageRatio.Inverted().ApplyTo(imageSize.width); gotHeight = true; } } diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index 36a43e46ba..21407511af 100644 --- a/layout/base/nsLayoutUtils.h +++ b/layout/base/nsLayoutUtils.h @@ -65,6 +65,7 @@ struct nsStyleImageOrientation; struct nsOverflowAreas; namespace mozilla { +struct aspectRatio; enum class CSSPseudoElementType : uint8_t; class EventListenerManager; class SVGImageContext; @@ -126,6 +127,7 @@ enum class RelativeTo { */ class nsLayoutUtils { + typedef mozilla::AspectRatio AspectRatio; typedef mozilla::dom::DOMRectList DOMRectList; typedef mozilla::layers::Layer Layer; typedef mozilla::ContainerLayerParameters ContainerLayerParameters; @@ -1217,7 +1219,7 @@ public: */ static nsRect ComputeObjectDestRect(const nsRect& aConstraintRect, const IntrinsicSize& aIntrinsicSize, - const nsSize& aIntrinsicRatio, + const AspectRatio& aIntrinsicRatio, const nsStylePosition* aStylePos, nsPoint* aAnchorPoint = nullptr); @@ -1837,7 +1839,7 @@ public: */ static void ComputeSizeForDrawing(imgIContainer* aImage, CSSIntSize& aImageSize, - nsSize& aIntrinsicRatio, + AspectRatio& aIntrinsicRatio, bool& aGotWidth, bool& aGotHeight); diff --git a/layout/generic/AspectRatio.h b/layout/generic/AspectRatio.h new file mode 100644 index 0000000000..0056c06205 --- /dev/null +++ b/layout/generic/AspectRatio.h @@ -0,0 +1,81 @@ +/* -*- 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_AspectRatio_h +#define mozilla_AspectRatio_h + +/* The aspect ratio of a box, in a "width / height" format. */ + +#include "mozilla/Attributes.h" +#include "nsCoord.h" +#include +#include + +namespace mozilla { + +struct AspectRatio { + AspectRatio() : mRatio(0.0f) {} + explicit AspectRatio(float aRatio) : mRatio(std::max(aRatio, 0.0f)) {} + + static AspectRatio FromSize(float aWidth, float aHeight) { + if (aWidth == 0.0f || aHeight == 0.0f) { + return AspectRatio(); + } + return AspectRatio(aWidth / aHeight); + } + + static AspectRatio FromSize(nsSize aSize) { + return FromSize(aSize.width, aSize.height); + } + + static AspectRatio FromSize(nsIntSize aSize) { + return FromSize(aSize.width, aSize.height); + } + + explicit operator bool() const { return mRatio != 0.0f; } + + nscoord ApplyTo(nscoord aCoord) const { + MOZ_DIAGNOSTIC_ASSERT(*this); + return NSCoordSaturatingNonnegativeMultiply(aCoord, mRatio); + } + + float ApplyToFloat(float aFloat) const { + MOZ_DIAGNOSTIC_ASSERT(*this); + return mRatio * aFloat; + } + + // Inverts the ratio, in order to get the height / width ratio. + MOZ_MUST_USE AspectRatio Inverted() const { + if (!*this) { + return AspectRatio(); + } + // Clamp to a small epsilon, in case mRatio is absurdly large & produces + // 0.0f in the division here (so that valid ratios always generate other + // valid ratios when inverted). + return AspectRatio( + std::max(std::numeric_limits::epsilon(), 1.0f / mRatio)); + } + + bool operator==(const AspectRatio& aOther) const { + return mRatio == aOther.mRatio; + } + + bool operator!=(const AspectRatio& aOther) const { + return !(*this == aOther); + } + + bool operator<(const AspectRatio& aOther) const { + return mRatio < aOther.mRatio; + } + + private: + // 0.0f represents no aspect ratio. + float mRatio; +}; + +} // namespace mozilla + +#endif // mozilla_AspectRatio_h diff --git a/layout/generic/moz.build b/layout/generic/moz.build index ad186ef7ae..b830470a32 100644 --- a/layout/generic/moz.build +++ b/layout/generic/moz.build @@ -109,6 +109,7 @@ EXPORTS += [ ] EXPORTS.mozilla += [ + 'AspectRatio.h', 'CSSAlignUtils.h', 'ReflowInput.h', 'ReflowOutput.h', diff --git a/layout/generic/nsFlexContainerFrame.cpp b/layout/generic/nsFlexContainerFrame.cpp index 69200117b0..a76097e1ef 100644 --- a/layout/generic/nsFlexContainerFrame.cpp +++ b/layout/generic/nsFlexContainerFrame.cpp @@ -557,8 +557,8 @@ public: return mFlexShrink * mFlexBaseSize; } - const nsSize& IntrinsicRatio() const { return mIntrinsicRatio; } - bool HasIntrinsicRatio() const { return mIntrinsicRatio != nsSize(); } + const AspectRatio IntrinsicRatio() const { return mIntrinsicRatio; } + bool HasIntrinsicRatio() const { return !!mIntrinsicRatio; } // Getters for margin: // =================== @@ -756,7 +756,7 @@ protected: const float mFlexGrow; const float mFlexShrink; - const nsSize mIntrinsicRatio; + const AspectRatio mIntrinsicRatio; const nsMargin mBorderPadding; nsMargin mMargin; // non-const because we need to resolve auto margins @@ -1520,22 +1520,20 @@ CrossSizeToUseWithRatio(const FlexItem& aFlexItem, } // Convenience function; returns a main-size, given a cross-size and an -// intrinsic ratio. The intrinsic ratio must not have 0 in its cross-axis -// component (or else we'll divide by 0). +// intrinsic ratio. The caller is responsible for ensuring that the passed-in +// intrinsic ratio is not zero. static nscoord MainSizeFromAspectRatio(nscoord aCrossSize, - const nsSize& aIntrinsicRatio, + const AspectRatio& aIntrinsicRatio, const FlexboxAxisTracker& aAxisTracker) { - MOZ_ASSERT(aAxisTracker.GetCrossComponent(aIntrinsicRatio) != 0, + MOZ_ASSERT(aIntrinsicRatio, "Invalid ratio; will divide by 0! Caller should've checked..."); + AspectRatio ratio = aAxisTracker.IsMainAxisHorizontal() + ? aIntrinsicRatio + : aIntrinsicRatio.Inverted(); - if (aAxisTracker.IsCrossAxisHorizontal()) { - // cross axis horiz --> aCrossSize is a width. Converting to height. - return NSCoordMulDiv(aCrossSize, aIntrinsicRatio.height, aIntrinsicRatio.width); - } - // cross axis vert --> aCrossSize is a height. Converting to width. - return NSCoordMulDiv(aCrossSize, aIntrinsicRatio.width, aIntrinsicRatio.height); + return ratio.ApplyTo(aCrossSize); } // Partially resolves "min-[width|height]:auto" and returns the resulting value. @@ -1584,7 +1582,7 @@ PartiallyResolveAutoMinSize(const FlexItem& aFlexItem, // * if the item has an intrinsic aspect ratio, the width (height) calculated // from the aspect ratio and any definite size constraints in the opposite // dimension. - if (aAxisTracker.GetCrossComponent(aFlexItem.IntrinsicRatio()) != 0) { + if (aFlexItem.IntrinsicRatio()) { // We have a usable aspect ratio. (not going to divide by 0) const bool useMinSizeIfCrossSizeIsIndefinite = true; nscoord crossSizeToUseWithRatio = @@ -1617,7 +1615,7 @@ ResolveAutoFlexBasisFromRatio(FlexItem& aFlexItem, // - a definite cross size // then the flex base size is calculated from its inner cross size and the // flex item’s intrinsic aspect ratio. - if (aAxisTracker.GetCrossComponent(aFlexItem.IntrinsicRatio()) != 0) { + if (aFlexItem.IntrinsicRatio()) { // We have a usable aspect ratio. (not going to divide by 0) const bool useMinSizeIfCrossSizeIsIndefinite = false; nscoord crossSizeToUseWithRatio = @@ -1693,8 +1691,7 @@ nsFlexContainerFrame:: // (We'll consider that later, if we need to.) resolvedMinSize = PartiallyResolveAutoMinSize(aFlexItem, aItemReflowInput, aAxisTracker); - if (resolvedMinSize > 0 && - aAxisTracker.GetCrossComponent(aFlexItem.IntrinsicRatio()) == 0) { + if (resolvedMinSize > 0 && !aFlexItem.IntrinsicRatio()) { // We don't have a usable aspect ratio, so we need to consider our // min-content size as another candidate min-size, which we'll have to // min() with the current resolvedMinSize. diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index dd7933b03e..afbd57f526 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -4641,10 +4641,10 @@ nsFrame::GetIntrinsicSize() return IntrinsicSize(); // default is width/height set to eStyleUnit_None } -/* virtual */ nsSize +/* virtual */ AspectRatio nsFrame::GetIntrinsicRatio() { - return nsSize(0, 0); + return AspectRatio(); } /* virtual */ @@ -4658,7 +4658,7 @@ nsFrame::ComputeSize(nsRenderingContext* aRenderingContext, const LogicalSize& aPadding, ComputeSizeFlags aFlags) { - MOZ_ASSERT(GetIntrinsicRatio() == nsSize(0,0), + MOZ_ASSERT(!GetIntrinsicRatio(), "Please override this method and call " "nsFrame::ComputeSizeWithIntrinsicDimensions instead."); LogicalSize result = ComputeAutoSize(aRenderingContext, aWM, @@ -4914,13 +4914,15 @@ LogicalSize nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingContext, WritingMode aWM, const IntrinsicSize& aIntrinsicSize, - nsSize aIntrinsicRatio, + const AspectRatio& aIntrinsicRatio, const LogicalSize& aCBSize, const LogicalSize& aMargin, const LogicalSize& aBorder, const LogicalSize& aPadding, ComputeSizeFlags aFlags) { + auto logicalRatio = + aWM.IsVertical() ? aIntrinsicRatio.Inverted() : aIntrinsicRatio; const nsStylePosition* stylePos = StylePosition(); const nsStyleCoord* inlineStyleCoord = &stylePos->ISize(aWM); const nsStyleCoord* blockStyleCoord = &stylePos->BSize(aWM); @@ -5181,10 +5183,6 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingConte intrinsicBSize = 0; } - NS_ASSERTION(aIntrinsicRatio.width >= 0 && aIntrinsicRatio.height >= 0, - "Intrinsic ratio has a negative component!"); - LogicalSize logicalRatio(aWM, aIntrinsicRatio); - // Now calculate the used values for iSize and bSize: if (isAutoISize) { @@ -5198,9 +5196,29 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingConte if (hasIntrinsicISize) { tentISize = intrinsicISize; - } else if (hasIntrinsicBSize && logicalRatio.BSize(aWM) > 0) { - tentISize = NSCoordMulDiv(intrinsicBSize, logicalRatio.ISize(aWM), logicalRatio.BSize(aWM)); - } else if (logicalRatio.ISize(aWM) > 0) { + } else if (hasIntrinsicBSize && logicalRatio) { + // (dholbert) + // This is wrong -- this ApplyTo call (and probably every ApplyTo call + // in this function) would only be valid if we're in a horizontal + // writing mode. It's not valid in a vertical writing mode. If this + // doesn't break tests, that's a bit concerning, and I think it means + // we're missing some test coverage. (That, or I'm misreading things.) + // + // aIntrinsicRatio is stored in terms of physical axes (width/height), + // either of which could be I vs. B axis. So any sort of + // aIntrinsicRatio.ApplyTo(someBSize) operation will be + // potentially-bogus. + // + // You probably want to bring back a logicalRatio variable + // (like the one we used to have here), but now with type AspectRatio. + // It would be equal to either aIntrinsicRatio or + // aIntrinsicRatio.Invert() depending on whether aWM is horizontal or + // vertical. (And hopefully having logical in its name would be a + // reminder that it's in terms of Inline/Block and can be used for + // these sorts of ApplyTo(intrinsicBSize) operations. + tentISize = logicalRatio.ApplyTo(intrinsicBSize); + } else if (logicalRatio) { tentISize = aCBSize.ISize(aWM) - boxSizingToMarginEdgeISize; // XXX scrollbar? if (tentISize < 0) tentISize = 0; } else { @@ -5217,8 +5235,8 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingConte if (hasIntrinsicBSize) { tentBSize = intrinsicBSize; - } else if (logicalRatio.ISize(aWM) > 0) { - tentBSize = NSCoordMulDiv(tentISize, logicalRatio.BSize(aWM), logicalRatio.ISize(aWM)); + } else if (logicalRatio) { + tentBSize = logicalRatio.Inverted().ApplyTo(tentISize); } else { tentBSize = nsPresContext::CSSPixelsToAppUnits(150); } @@ -5229,46 +5247,39 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingConte stretchB = (stretchI == eStretch ? eStretch : eStretchPreservingRatio); } - if (aIntrinsicRatio != nsSize(0, 0)) { + if (logicalRatio) { if (stretchI == eStretch) { tentISize = iSize; // * / 'stretch' if (stretchB == eStretch) { tentBSize = bSize; // 'stretch' / 'stretch' - } else if (stretchB == eStretchPreservingRatio && logicalRatio.ISize(aWM) > 0) { + } else if (stretchB == eStretchPreservingRatio) { // 'normal' / 'stretch' - tentBSize = NSCoordMulDiv(iSize, logicalRatio.BSize(aWM), logicalRatio.ISize(aWM)); + tentBSize = logicalRatio.Inverted().ApplyTo(iSize); } } else if (stretchB == eStretch) { tentBSize = bSize; // 'stretch' / * (except 'stretch') - if (stretchI == eStretchPreservingRatio && logicalRatio.BSize(aWM) > 0) { + if (stretchI == eStretchPreservingRatio) { // 'stretch' / 'normal' - tentISize = NSCoordMulDiv(bSize, logicalRatio.ISize(aWM), logicalRatio.BSize(aWM)); + tentISize = logicalRatio.ApplyTo(bSize); } } else if (stretchI == eStretchPreservingRatio) { tentISize = iSize; // * (except 'stretch') / 'normal' - if (logicalRatio.ISize(aWM) > 0) { - tentBSize = NSCoordMulDiv(iSize, logicalRatio.BSize(aWM), logicalRatio.ISize(aWM)); - } + tentBSize = logicalRatio.Inverted().ApplyTo(iSize); if (stretchB == eStretchPreservingRatio && tentBSize > bSize) { // Stretch within the CB size with preserved intrinsic ratio. tentBSize = bSize; // 'normal' / 'normal' - if (logicalRatio.BSize(aWM) > 0) { - tentISize = NSCoordMulDiv(bSize, logicalRatio.ISize(aWM), logicalRatio.BSize(aWM)); - } + tentISize = logicalRatio.ApplyTo(bSize); } } else if (stretchB == eStretchPreservingRatio) { tentBSize = bSize; // 'normal' / * (except 'normal' and 'stretch') - if (logicalRatio.BSize(aWM) > 0) { - tentISize = NSCoordMulDiv(bSize, logicalRatio.ISize(aWM), logicalRatio.BSize(aWM)); - } + tentISize = logicalRatio.ApplyTo(bSize); } } // ComputeAutoSizeWithIntrinsicDimensions preserves the ratio when applying // the min/max-size. We don't want that when we have 'stretch' in either // axis because tentISize/tentBSize is likely not according to ratio now. - if (aIntrinsicRatio != nsSize(0, 0) && - stretchI != eStretch && stretchB != eStretch) { + if (logicalRatio && stretchI != eStretch && stretchB != eStretch) { nsSize autoSize = nsLayoutUtils:: ComputeAutoSizeWithIntrinsicDimensions(minISize, minBSize, maxISize, maxBSize, @@ -5289,8 +5300,8 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingConte // 'auto' iSize, non-'auto' bSize bSize = NS_CSS_MINMAX(bSize, minBSize, maxBSize); if (stretchI != eStretch) { - if (logicalRatio.BSize(aWM) > 0) { - iSize = NSCoordMulDiv(bSize, logicalRatio.ISize(aWM), logicalRatio.BSize(aWM)); + if (logicalRatio) { + iSize = logicalRatio.ApplyTo(bSize); } else if (hasIntrinsicISize) { if (!((aFlags & ComputeSizeFlags::eIClampMarginBoxMinSize) && intrinsicISize > iSize)) { @@ -5309,8 +5320,8 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingConte // non-'auto' iSize, 'auto' bSize iSize = NS_CSS_MINMAX(iSize, minISize, maxISize); if (stretchB != eStretch) { - if (logicalRatio.ISize(aWM) > 0) { - bSize = NSCoordMulDiv(iSize, logicalRatio.BSize(aWM), logicalRatio.ISize(aWM)); + if (logicalRatio) { + bSize = logicalRatio.Inverted().ApplyTo(iSize); } else if (hasIntrinsicBSize) { if (!((aFlags & ComputeSizeFlags::eBClampMarginBoxMinSize) && intrinsicBSize > bSize)) { diff --git a/layout/generic/nsFrame.h b/layout/generic/nsFrame.h index 439e398562..d75555fec2 100644 --- a/layout/generic/nsFrame.h +++ b/layout/generic/nsFrame.h @@ -264,7 +264,7 @@ public: IntrinsicISizeOffsetData IntrinsicISizeOffsets(nscoord aPercentageBasis = NS_UNCONSTRAINEDSIZE) override; virtual mozilla::IntrinsicSize GetIntrinsicSize() override; - virtual nsSize GetIntrinsicRatio() override; + virtual mozilla::AspectRatio GetIntrinsicRatio() override; virtual mozilla::LogicalSize ComputeSize(nsRenderingContext* aRenderingContext, @@ -285,7 +285,7 @@ public: nsRenderingContext* aRenderingContext, mozilla::WritingMode aWM, const mozilla::IntrinsicSize& aIntrinsicSize, - nsSize aIntrinsicRatio, + const mozilla::AspectRatio& aIntrinsicRatio, const mozilla::LogicalSize& aCBSize, const mozilla::LogicalSize& aMargin, const mozilla::LogicalSize& aBorder, diff --git a/layout/generic/nsHTMLCanvasFrame.cpp b/layout/generic/nsHTMLCanvasFrame.cpp index f86ec11369..032b95e5bb 100644 --- a/layout/generic/nsHTMLCanvasFrame.cpp +++ b/layout/generic/nsHTMLCanvasFrame.cpp @@ -44,21 +44,6 @@ IntrinsicSizeFromCanvasSize(const nsIntSize& aCanvasSizeInPx) return intrinsicSize; } -/* Helper for our nsIFrame::GetIntrinsicRatio() impl. Takes the result of - * "GetCanvasSize()" as a parameter, which may help avoid redundant - * indirect calls to GetCanvasSize(). - * - * @param aCanvasSizeInPx The canvas's size in CSS pixels, as returned - * by GetCanvasSize(). - * @return The canvas's intrinsic ratio, as a nsSize. - */ -static nsSize -IntrinsicRatioFromCanvasSize(const nsIntSize& aCanvasSizeInPx) -{ - return nsSize(nsPresContext::CSSPixelsToAppUnits(aCanvasSizeInPx.width), - nsPresContext::CSSPixelsToAppUnits(aCanvasSizeInPx.height)); -} - class nsDisplayCanvas : public nsDisplayItem { public: nsDisplayCanvas(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame) @@ -92,7 +77,7 @@ public: // Need intrinsic size & ratio, for ComputeObjectDestRect: nsIntSize canvasSize = f->GetCanvasSize(); IntrinsicSize intrinsicSize = IntrinsicSizeFromCanvasSize(canvasSize); - nsSize intrinsicRatio = IntrinsicRatioFromCanvasSize(canvasSize); + AspectRatio intrinsicRatio = AspectRatio::FromSize(canvasSize); const nsRect destRect = nsLayoutUtils::ComputeObjectDestRect(constraintRect, @@ -211,10 +196,15 @@ nsHTMLCanvasFrame::GetIntrinsicSize() return IntrinsicSizeFromCanvasSize(GetCanvasSize()); } -/* virtual */ nsSize +/* virtual */ AspectRatio nsHTMLCanvasFrame::GetIntrinsicRatio() { - return IntrinsicRatioFromCanvasSize(GetCanvasSize()); +/* + if (StyleDisplay()->IsContainSize()) { + return AspectRatio(); + } + */ + return AspectRatio::FromSize(GetCanvasSize()); } /* virtual */ @@ -234,7 +224,7 @@ nsHTMLCanvasFrame::ComputeSize(nsRenderingContext *aRenderingContext, intrinsicSize.width.SetCoordValue(nsPresContext::CSSPixelsToAppUnits(size.width)); intrinsicSize.height.SetCoordValue(nsPresContext::CSSPixelsToAppUnits(size.height)); - nsSize intrinsicRatio = GetIntrinsicRatio(); // won't actually be used + AspectRatio intrinsicRatio = GetIntrinsicRatio(); return ComputeSizeWithIntrinsicDimensions(aRenderingContext, aWM, intrinsicSize, intrinsicRatio, @@ -340,7 +330,7 @@ nsHTMLCanvasFrame::BuildLayer(nsDisplayListBuilder* aBuilder, return nullptr; IntrinsicSize intrinsicSize = IntrinsicSizeFromCanvasSize(canvasSizeInPx); - nsSize intrinsicRatio = IntrinsicRatioFromCanvasSize(canvasSizeInPx); + AspectRatio intrinsicRatio = AspectRatio::FromSize(canvasSizeInPx); nsRect dest = nsLayoutUtils::ComputeObjectDestRect(area, intrinsicSize, intrinsicRatio, diff --git a/layout/generic/nsHTMLCanvasFrame.h b/layout/generic/nsHTMLCanvasFrame.h index b2d159627f..8432ad2247 100644 --- a/layout/generic/nsHTMLCanvasFrame.h +++ b/layout/generic/nsHTMLCanvasFrame.h @@ -58,7 +58,7 @@ public: virtual nscoord GetMinISize(nsRenderingContext *aRenderingContext) override; virtual nscoord GetPrefISize(nsRenderingContext *aRenderingContext) override; virtual mozilla::IntrinsicSize GetIntrinsicSize() override; - virtual nsSize GetIntrinsicRatio() override; + virtual mozilla::AspectRatio GetIntrinsicRatio() override; virtual mozilla::LogicalSize ComputeSize(nsRenderingContext *aRenderingContext, diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index dfe83bcbe1..d54ddc2b5b 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -27,6 +27,7 @@ #include "FrameProperties.h" #include "LayoutConstants.h" #include "mozilla/layout/FrameChildList.h" +#include "mozilla/AspectRatio.h" #include "mozilla/Maybe.h" #include "mozilla/WritingModes.h" #include "nsDirection.h" @@ -2063,15 +2064,14 @@ public: virtual mozilla::IntrinsicSize GetIntrinsicSize() = 0; /** - * Get the intrinsic ratio of this element, or nsSize(0,0) if it has - * no intrinsic ratio. The intrinsic ratio is the ratio of the - * height/width of a box with an intrinsic size or the intrinsic - * aspect ratio of a scalable vector image without an intrinsic size. + * Get the intrinsic ratio of this element, or a default-constructed + * AspectRatio if it has no intrinsic ratio. * - * Either one of the sides may be zero, indicating a zero or infinite - * ratio. + * The intrinsic ratio is the ratio of the width/height of a box with an + * intrinsic size or the intrinsic aspect ratio of a scalable vector image + * without an intrinsic size. */ - virtual nsSize GetIntrinsicRatio() = 0; + virtual mozilla::AspectRatio GetIntrinsicRatio() = 0; /** * Bit-flags to pass to ComputeSize in |aFlags| parameter. diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp index b96af66b54..365b7810b3 100644 --- a/layout/generic/nsImageFrame.cpp +++ b/layout/generic/nsImageFrame.cpp @@ -138,7 +138,6 @@ NS_IMPL_FRAMEARENA_HELPERS(nsImageFrame) nsImageFrame::nsImageFrame(nsStyleContext* aContext) : nsAtomicContainerFrame(aContext), mComputedSize(0, 0), - mIntrinsicRatio(0, 0), mDisplayingIcon(false), mFirstFrameComplete(false), mReflowCallbackPosted(false), @@ -325,11 +324,11 @@ nsImageFrame::UpdateIntrinsicRatio(imgIContainer* aImage) if (!aImage) return false; - nsSize oldIntrinsicRatio = mIntrinsicRatio; + AspectRatio oldIntrinsicRatio = mIntrinsicRatio; // Set intrinsic ratio to match aImage's reported intrinsic ratio. if (NS_FAILED(aImage->GetIntrinsicRatio(&mIntrinsicRatio))) - mIntrinsicRatio.SizeTo(0, 0); + mIntrinsicRatio = AspectRatio(); return mIntrinsicRatio != oldIntrinsicRatio; } @@ -557,7 +556,7 @@ nsImageFrame::OnSizeAvailable(imgIRequest* aRequest, imgIContainer* aImage) // Have to size to 0,0 so that GetDesiredSize recalculates the size. mIntrinsicSize.width.SetCoordValue(0); mIntrinsicSize.height.SetCoordValue(0); - mIntrinsicRatio.SizeTo(0, 0); + mIntrinsicRatio = AspectRatio(); intrinsicSizeChanged = true; } @@ -672,7 +671,7 @@ nsImageFrame::NotifyNewCurrentRequest(imgIRequest *aRequest, // Have to size to 0,0 so that GetDesiredSize recalculates the size mIntrinsicSize.width.SetCoordValue(0); mIntrinsicSize.height.SetCoordValue(0); - mIntrinsicRatio.SizeTo(0, 0); + mIntrinsicRatio = AspectRatio(); } if (mState & IMAGE_GOTINITIALREFLOW) { // do nothing if we haven't gotten the initial reflow yet @@ -808,7 +807,7 @@ nsImageFrame::EnsureIntrinsicSizeAndRatio() ICON_SIZE + (2 * (ICON_PADDING + ALT_BORDER_WIDTH))); mIntrinsicSize.width.SetCoordValue(edgeLengthToUse); mIntrinsicSize.height.SetCoordValue(edgeLengthToUse); - mIntrinsicRatio.SizeTo(1, 1); + mIntrinsicRatio = AspectRatio(1.0f); } } } @@ -932,7 +931,7 @@ nsImageFrame::GetIntrinsicSize() return mIntrinsicSize; } -/* virtual */ nsSize +/* virtual */ AspectRatio nsImageFrame::GetIntrinsicRatio() { return mIntrinsicRatio; diff --git a/layout/generic/nsImageFrame.h b/layout/generic/nsImageFrame.h index 59af1be328..2414d89dfc 100644 --- a/layout/generic/nsImageFrame.h +++ b/layout/generic/nsImageFrame.h @@ -86,7 +86,7 @@ public: virtual nscoord GetMinISize(nsRenderingContext *aRenderingContext) override; virtual nscoord GetPrefISize(nsRenderingContext *aRenderingContext) override; virtual mozilla::IntrinsicSize GetIntrinsicSize() override; - virtual nsSize GetIntrinsicRatio() override; + virtual mozilla::AspectRatio GetIntrinsicRatio() override; virtual void Reflow(nsPresContext* aPresContext, ReflowOutput& aDesiredSize, const ReflowInput& aReflowInput, @@ -333,7 +333,7 @@ private: nsCOMPtr mPrevImage; nsSize mComputedSize; mozilla::IntrinsicSize mIntrinsicSize; - nsSize mIntrinsicRatio; + mozilla::AspectRatio mIntrinsicRatio; bool mDisplayingIcon; bool mFirstFrameComplete; diff --git a/layout/generic/nsLineLayout.cpp b/layout/generic/nsLineLayout.cpp index 6a15a9cfa8..60e4e8c964 100644 --- a/layout/generic/nsLineLayout.cpp +++ b/layout/generic/nsLineLayout.cpp @@ -784,7 +784,7 @@ IsPercentageAware(const nsIFrame* aFrame) // is calculated from the constraint equation used for // block-level, non-replaced elements in normal flow. nsIFrame *f = const_cast(aFrame); - if (f->GetIntrinsicRatio() != nsSize(0, 0) && + if (f->GetIntrinsicRatio() && // Some percents are treated like 'auto', so check != coord pos->mHeight.GetUnit() != eStyleUnit_Coord) { const IntrinsicSize &intrinsicSize = f->GetIntrinsicSize(); diff --git a/layout/generic/nsSubDocumentFrame.cpp b/layout/generic/nsSubDocumentFrame.cpp index fd9a7d32cb..31d04a3351 100644 --- a/layout/generic/nsSubDocumentFrame.cpp +++ b/layout/generic/nsSubDocumentFrame.cpp @@ -671,7 +671,7 @@ nsSubDocumentFrame::GetIntrinsicSize() return nsAtomicContainerFrame::GetIntrinsicSize(); } -/* virtual */ nsSize +/* virtual */ AspectRatio nsSubDocumentFrame::GetIntrinsicRatio() { nsIFrame* subDocRoot = ObtainIntrinsicSizeFrame(); @@ -771,7 +771,7 @@ nsSubDocumentFrame::Reflow(nsPresContext* aPresContext, // Size & position the view according to 'object-fit' & 'object-position'. nsIFrame* subDocRoot = ObtainIntrinsicSizeFrame(); IntrinsicSize intrinsSize; - nsSize intrinsRatio; + AspectRatio intrinsRatio; if (subDocRoot) { intrinsSize = subDocRoot->GetIntrinsicSize(); intrinsRatio = subDocRoot->GetIntrinsicRatio(); diff --git a/layout/generic/nsSubDocumentFrame.h b/layout/generic/nsSubDocumentFrame.h index 93d908dcc5..907d337106 100644 --- a/layout/generic/nsSubDocumentFrame.h +++ b/layout/generic/nsSubDocumentFrame.h @@ -51,7 +51,7 @@ public: virtual nscoord GetPrefISize(nsRenderingContext *aRenderingContext) override; virtual mozilla::IntrinsicSize GetIntrinsicSize() override; - virtual nsSize GetIntrinsicRatio() override; + virtual mozilla::AspectRatio GetIntrinsicRatio() override; virtual mozilla::LogicalSize ComputeAutoSize(nsRenderingContext* aRenderingContext, diff --git a/layout/generic/nsVideoFrame.cpp b/layout/generic/nsVideoFrame.cpp index cea209cb5a..6bd6395a32 100644 --- a/layout/generic/nsVideoFrame.cpp +++ b/layout/generic/nsVideoFrame.cpp @@ -217,12 +217,10 @@ nsVideoFrame::BuildLayer(nsDisplayListBuilder* aBuilder, // Convert video size from pixel units into app units, to get an aspect-ratio // (which has to be represented as a nsSize) and an IntrinsicSize that we // can pass to ComputeObjectRenderRect. - nsSize aspectRatio(nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.width), - nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.height)); + auto aspectRatio = AspectRatio::FromSize(videoSizeInPx); IntrinsicSize intrinsicSize; - intrinsicSize.width.SetCoordValue(aspectRatio.width); - intrinsicSize.height.SetCoordValue(aspectRatio.height); - + intrinsicSize.width.SetCoordValue(nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.width)); + intrinsicSize.height.SetCoordValue(nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.height)); nsRect dest = nsLayoutUtils::ComputeObjectDestRect(area, intrinsicSize, aspectRatio, @@ -533,7 +531,9 @@ nsVideoFrame::ComputeSize(nsRenderingContext *aRenderingContext, intrinsicSize.height.SetCoordValue(size.height); // Only video elements have an intrinsic ratio. - nsSize intrinsicRatio = HasVideoElement() ? size : nsSize(0, 0); + auto intrinsicRatio = HasVideoElement() + ? AspectRatio::FromSize(size) + : AspectRatio(); return ComputeSizeWithIntrinsicDimensions(aRenderingContext, aWM, intrinsicSize, intrinsicRatio, @@ -557,14 +557,14 @@ nscoord nsVideoFrame::GetPrefISize(nsRenderingContext *aRenderingContext) return result; } -nsSize nsVideoFrame::GetIntrinsicRatio() +AspectRatio nsVideoFrame::GetIntrinsicRatio() { if (!HasVideoElement()) { // Audio elements have no intrinsic ratio. - return nsSize(0, 0); + return AspectRatio(); } - return GetVideoIntrinsicSize(nullptr); + return AspectRatio::FromSize(GetVideoIntrinsicSize(nullptr)); } bool nsVideoFrame::ShouldDisplayPoster() diff --git a/layout/generic/nsVideoFrame.h b/layout/generic/nsVideoFrame.h index d624ae6b90..f52cd82779 100644 --- a/layout/generic/nsVideoFrame.h +++ b/layout/generic/nsVideoFrame.h @@ -56,7 +56,7 @@ public: /* get the size of the video's display */ nsSize GetVideoIntrinsicSize(nsRenderingContext *aRenderingContext); - virtual nsSize GetIntrinsicRatio() override; + virtual mozilla::AspectRatio GetIntrinsicRatio() override; virtual mozilla::LogicalSize ComputeSize(nsRenderingContext *aRenderingContext, mozilla::WritingMode aWritingMode, diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 200f934c5e..9270f29600 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -2731,7 +2731,7 @@ nsStyleImageLayers::Size::DependsOnPositioningAreaSize(const nsStyleImage& aImag } if (imgContainer) { CSSIntSize imageSize; - nsSize imageRatio; + AspectRatio imageRatio; bool hasWidth, hasHeight; nsLayoutUtils::ComputeSizeForDrawing(imgContainer, imageSize, imageRatio, hasWidth, hasHeight); @@ -2744,7 +2744,7 @@ nsStyleImageLayers::Size::DependsOnPositioningAreaSize(const nsStyleImage& aImag // If the image has an intrinsic ratio, rendering will depend on frame // size when background-size is all auto. - if (imageRatio != nsSize(0, 0)) { + if (imageRatio) { return mWidthType == mHeightType; } diff --git a/layout/svg/nsSVGOuterSVGFrame.cpp b/layout/svg/nsSVGOuterSVGFrame.cpp index 16dfc2b37a..d8cd2cb771 100644 --- a/layout/svg/nsSVGOuterSVGFrame.cpp +++ b/layout/svg/nsSVGOuterSVGFrame.cpp @@ -232,28 +232,38 @@ nsSVGOuterSVGFrame::GetIntrinsicSize() return intrinsicSize; } -/* virtual */ nsSize +/* virtual */ AspectRatio nsSVGOuterSVGFrame::GetIntrinsicRatio() { + // 2020-07-14 (RealityRipple) Firefox Uses a new IsReplacedAndContainSize(this) + // function call [Line 96-99 on trunk]. +/* + static inline bool IsReplacedAndContainSize(const nsSVGOuterSVGFrame* aFrame) { + return aFrame->GetContent->GetParent() && + aFrame->StyleDisplay()->IsContainSize(); + } + */ + // but since contain: size doesn't exist in Pale Moon yet... +/* + if (IsReplacedAndContainSize(this)) { + return AspectRatio(); + } + */ + // We only have an intrinsic size/ratio if our width and height attributes // are both specified and set to non-percentage values, or we have a viewBox // rect: http://www.w3.org/TR/SVGMobile12/coords.html#IntrinsicSizing + // Unfortunately we have to return the ratio as two nscoords whereas what + // we have are two floats. Using app units allows for some floating point + // values to work but really small or large numbers will fail. SVGSVGElement *content = static_cast(mContent); nsSVGLength2 &width = content->mLengthAttributes[SVGSVGElement::ATTR_WIDTH]; nsSVGLength2 &height = content->mLengthAttributes[SVGSVGElement::ATTR_HEIGHT]; if (!width.IsPercentage() && !height.IsPercentage()) { - nsSize ratio( - nsPresContext::CSSPixelsToAppUnits(width.GetAnimValue(content)), - nsPresContext::CSSPixelsToAppUnits(height.GetAnimValue(content))); - if (ratio.width < 0) { - ratio.width = 0; - } - if (ratio.height < 0) { - ratio.height = 0; - } - return ratio; + return AspectRatio::FromSize(width.GetAnimValue(content), + height.GetAnimValue(content)); } SVGViewElement* viewElement = content->GetCurrentViewElement(); @@ -267,17 +277,7 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio() } if (viewbox) { - float viewBoxWidth = viewbox->width; - float viewBoxHeight = viewbox->height; - - if (viewBoxWidth < 0.0f) { - viewBoxWidth = 0.0f; - } - if (viewBoxHeight < 0.0f) { - viewBoxHeight = 0.0f; - } - return nsSize(nsPresContext::CSSPixelsToAppUnits(viewBoxWidth), - nsPresContext::CSSPixelsToAppUnits(viewBoxHeight)); + return AspectRatio::FromSize(viewbox->width, viewbox->height); } return nsSVGDisplayContainerFrame::GetIntrinsicRatio(); diff --git a/layout/svg/nsSVGOuterSVGFrame.h b/layout/svg/nsSVGOuterSVGFrame.h index ee59b7d1c5..4705b6ebe7 100644 --- a/layout/svg/nsSVGOuterSVGFrame.h +++ b/layout/svg/nsSVGOuterSVGFrame.h @@ -42,7 +42,7 @@ public: virtual nscoord GetPrefISize(nsRenderingContext *aRenderingContext) override; virtual mozilla::IntrinsicSize GetIntrinsicSize() override; - virtual nsSize GetIntrinsicRatio() override; + virtual mozilla::AspectRatio GetIntrinsicRatio() override; virtual mozilla::LogicalSize ComputeSize(nsRenderingContext *aRenderingContext, diff --git a/layout/xul/nsImageBoxFrame.cpp b/layout/xul/nsImageBoxFrame.cpp index e88b502ab7..5286b6d883 100644 --- a/layout/xul/nsImageBoxFrame.cpp +++ b/layout/xul/nsImageBoxFrame.cpp @@ -402,12 +402,13 @@ nsImageBoxFrame::PaintImage(nsRenderingContext& aRenderingContext, // Determine dest rect based on intrinsic size & ratio, along with // 'object-fit' & 'object-position' properties: IntrinsicSize intrinsicSize; - nsSize intrinsicRatio; + AspectRatio intrinsicRatio; if (mIntrinsicSize.width > 0 && mIntrinsicSize.height > 0) { // Image has a valid size; use it as intrinsic size & ratio. intrinsicSize.width.SetCoordValue(mIntrinsicSize.width); intrinsicSize.height.SetCoordValue(mIntrinsicSize.height); - intrinsicRatio = mIntrinsicSize; + intrinsicRatio = + AspectRatio::FromSize(mIntrinsicSize); } else { // Image doesn't have a (valid) intrinsic size. // Try to look up intrinsic ratio and use that at least. From 777b3cc6227f7b540f02b3deeb0a051fe49ace56 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 31 Jul 2020 13:01:58 -0700 Subject: [PATCH 04/16] Issue #1619 - Missing Dimension Computation This existed in Firefox before this bug. I don't know if it came from a previous bug or was removed post-fork. --- layout/base/nsCSSRendering.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp index e6e3b5f6fe..d5df170203 100644 --- a/layout/base/nsCSSRendering.cpp +++ b/layout/base/nsCSSRendering.cpp @@ -5290,6 +5290,16 @@ nsImageRenderer::ComputeIntrinsicSize() if (haveHeight) { result.SetHeight(nsPresContext::CSSPixelsToAppUnits(imageIntSize.height)); } + + if (!haveHeight && haveWidth && result.mRatio) { + nscoord intrinsicHeight = + result.mRatio.Inverted().ApplyTo(imageIntSize.width); + result.SetHeight(nsPresContext::CSSPixelsToAppUnits(intrinsicHeight)); + } else if (haveHeight && !haveWidth && result.mRatio) { + nscoord intrinsicWidth = result.mRatio.ApplyTo(imageIntSize.height); + result.SetWidth(nsPresContext::CSSPixelsToAppUnits(intrinsicWidth)); + } + break; } case eStyleImageType_Element: From 217374ebf67cd853a0f8fcd0cf6c9e9fd6f4fb38 Mon Sep 17 00:00:00 2001 From: Andy Date: Sat, 1 Aug 2020 20:54:54 -0700 Subject: [PATCH 05/16] Issue #1619 - Add Vertical Writing Testcase Ensures aspect ratio numerator and denominator aren't swapped in vertical writing modes. https://bugzilla.mozilla.org/show_bug.cgi?id=1548768 --- layout/generic/nsFrame.cpp | 20 ------- .../object_intrinsic_size_vertical.html | 57 +++++++++++++++++++ 2 files changed, 57 insertions(+), 20 deletions(-) create mode 100644 testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_intrinsic_size_vertical.html diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index afbd57f526..72923c4b7f 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -5197,26 +5197,6 @@ nsFrame::ComputeSizeWithIntrinsicDimensions(nsRenderingContext* aRenderingConte if (hasIntrinsicISize) { tentISize = intrinsicISize; } else if (hasIntrinsicBSize && logicalRatio) { - // (dholbert) - // This is wrong -- this ApplyTo call (and probably every ApplyTo call - // in this function) would only be valid if we're in a horizontal - // writing mode. It's not valid in a vertical writing mode. If this - // doesn't break tests, that's a bit concerning, and I think it means - // we're missing some test coverage. (That, or I'm misreading things.) - // - // aIntrinsicRatio is stored in terms of physical axes (width/height), - // either of which could be I vs. B axis. So any sort of - // aIntrinsicRatio.ApplyTo(someBSize) operation will be - // potentially-bogus. - // - // You probably want to bring back a logicalRatio variable - // (like the one we used to have here), but now with type AspectRatio. - // It would be equal to either aIntrinsicRatio or - // aIntrinsicRatio.Invert() depending on whether aWM is horizontal or - // vertical. (And hopefully having logical in its name would be a - // reminder that it's in terms of Inline/Block and can be used for - // these sorts of ApplyTo(intrinsicBSize) operations. tentISize = logicalRatio.ApplyTo(intrinsicBSize); } else if (logicalRatio) { tentISize = aCBSize.ISize(aWM) - boxSizingToMarginEdgeISize; // XXX scrollbar? diff --git a/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_intrinsic_size_vertical.html b/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_intrinsic_size_vertical.html new file mode 100644 index 0000000000..0dca4c377d --- /dev/null +++ b/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_intrinsic_size_vertical.html @@ -0,0 +1,57 @@ + +Frame width and height attributes as they apply in a vertical writing mode + + + +
+ + +
+
+ + +
+ From f15f44b4452e074bcdeff5b58b7eee9a13c2cdab Mon Sep 17 00:00:00 2001 From: Andy Date: Sun, 2 Aug 2020 08:01:58 -0700 Subject: [PATCH 06/16] Issue #1619 - Nits Picked --- layout/generic/nsFlexContainerFrame.cpp | 6 +++--- layout/generic/nsHTMLCanvasFrame.cpp | 1 + layout/generic/nsVideoFrame.cpp | 6 +++--- layout/xul/nsImageBoxFrame.cpp | 3 +-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/layout/generic/nsFlexContainerFrame.cpp b/layout/generic/nsFlexContainerFrame.cpp index a76097e1ef..a03d777e7e 100644 --- a/layout/generic/nsFlexContainerFrame.cpp +++ b/layout/generic/nsFlexContainerFrame.cpp @@ -1529,9 +1529,9 @@ MainSizeFromAspectRatio(nscoord aCrossSize, { MOZ_ASSERT(aIntrinsicRatio, "Invalid ratio; will divide by 0! Caller should've checked..."); - AspectRatio ratio = aAxisTracker.IsMainAxisHorizontal() - ? aIntrinsicRatio - : aIntrinsicRatio.Inverted(); + AspectRatio ratio = aAxisTracker.IsMainAxisHorizontal() ? + aIntrinsicRatio : + aIntrinsicRatio.Inverted(); return ratio.ApplyTo(aCrossSize); } diff --git a/layout/generic/nsHTMLCanvasFrame.cpp b/layout/generic/nsHTMLCanvasFrame.cpp index 032b95e5bb..f40f799bab 100644 --- a/layout/generic/nsHTMLCanvasFrame.cpp +++ b/layout/generic/nsHTMLCanvasFrame.cpp @@ -199,6 +199,7 @@ nsHTMLCanvasFrame::GetIntrinsicSize() /* virtual */ AspectRatio nsHTMLCanvasFrame::GetIntrinsicRatio() { + // When 'contain: size' is implemented, make sure to check for it. /* if (StyleDisplay()->IsContainSize()) { return AspectRatio(); diff --git a/layout/generic/nsVideoFrame.cpp b/layout/generic/nsVideoFrame.cpp index 6bd6395a32..0472ebe626 100644 --- a/layout/generic/nsVideoFrame.cpp +++ b/layout/generic/nsVideoFrame.cpp @@ -531,9 +531,9 @@ nsVideoFrame::ComputeSize(nsRenderingContext *aRenderingContext, intrinsicSize.height.SetCoordValue(size.height); // Only video elements have an intrinsic ratio. - auto intrinsicRatio = HasVideoElement() - ? AspectRatio::FromSize(size) - : AspectRatio(); + auto intrinsicRatio = HasVideoElement() ? + AspectRatio::FromSize(size) : + AspectRatio(); return ComputeSizeWithIntrinsicDimensions(aRenderingContext, aWM, intrinsicSize, intrinsicRatio, diff --git a/layout/xul/nsImageBoxFrame.cpp b/layout/xul/nsImageBoxFrame.cpp index 5286b6d883..aa00c5174d 100644 --- a/layout/xul/nsImageBoxFrame.cpp +++ b/layout/xul/nsImageBoxFrame.cpp @@ -407,8 +407,7 @@ nsImageBoxFrame::PaintImage(nsRenderingContext& aRenderingContext, // Image has a valid size; use it as intrinsic size & ratio. intrinsicSize.width.SetCoordValue(mIntrinsicSize.width); intrinsicSize.height.SetCoordValue(mIntrinsicSize.height); - intrinsicRatio = - AspectRatio::FromSize(mIntrinsicSize); + intrinsicRatio = AspectRatio::FromSize(mIntrinsicSize); } else { // Image doesn't have a (valid) intrinsic size. // Try to look up intrinsic ratio and use that at least. From b52bc1586d635f95a6d64d3ff5d324b9ccbb36ce Mon Sep 17 00:00:00 2001 From: Moonchild Date: Mon, 3 Aug 2020 09:41:42 +0000 Subject: [PATCH 07/16] [js] Try to catch bad pointers for GC and bail if not valid. --- js/public/HeapAPI.h | 37 +++++++++++++++++++++++++++++++++++++ js/src/gc/Marking.cpp | 5 +++++ 2 files changed, 42 insertions(+) diff --git a/js/public/HeapAPI.h b/js/public/HeapAPI.h index fef6c0c780..d033d3706e 100644 --- a/js/public/HeapAPI.h +++ b/js/public/HeapAPI.h @@ -51,6 +51,7 @@ const size_t ChunkMarkBitmapBits = 129024; const size_t ChunkRuntimeOffset = ChunkSize - sizeof(void*); const size_t ChunkTrailerSize = 2 * sizeof(uintptr_t) + sizeof(uint64_t); const size_t ChunkLocationOffset = ChunkSize - ChunkTrailerSize; +const size_t ChunkStoreBufferOffset = ChunkSize - ChunkTrailerSize + sizeof(uint64_t); const size_t ArenaZoneOffset = sizeof(size_t); const size_t ArenaHeaderSize = sizeof(size_t) + 2 * sizeof(uintptr_t) + sizeof(size_t) + sizeof(uintptr_t); @@ -326,6 +327,20 @@ CellIsMarkedGray(const Cell* cell) extern JS_PUBLIC_API(bool) CellIsMarkedGrayIfKnown(const Cell* cell); +MOZ_ALWAYS_INLINE ChunkLocation GetCellLocation(const void* cell) { + uintptr_t addr = uintptr_t(cell); + addr &= ~js::gc::ChunkMask; + addr |= js::gc::ChunkLocationOffset; + return *reinterpret_cast(addr); +} + +MOZ_ALWAYS_INLINE bool NurseryCellHasStoreBuffer(const void* cell) { + uintptr_t addr = uintptr_t(cell); + addr &= ~js::gc::ChunkMask; + addr |= js::gc::ChunkStoreBufferOffset; + return *reinterpret_cast(addr) != nullptr; +} + } /* namespace detail */ MOZ_ALWAYS_INLINE bool @@ -341,6 +356,28 @@ IsInsideNursery(const js::gc::Cell* cell) return location == ChunkLocation::Nursery; } +MOZ_ALWAYS_INLINE bool IsCellPointerValid(const void* cell) { + auto addr = uintptr_t(cell); + if (addr < ChunkSize || addr % CellSize != 0) { + return false; + } + auto location = detail::GetCellLocation(cell); + if (location == ChunkLocation::TenuredHeap) { + return !!detail::GetGCThingZone(addr); + } + if (location == ChunkLocation::Nursery) { + return detail::NurseryCellHasStoreBuffer(cell); + } + return false; +} + +MOZ_ALWAYS_INLINE bool IsCellPointerValidOrNull(const void* cell) { + if (!cell) { + return true; + } + return IsCellPointerValid(cell); +} + } /* namespace gc */ } /* namespace js */ diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 43e325394d..b4db0297a6 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -2267,6 +2267,8 @@ void js::gc::StoreBuffer::SlotsEdge::trace(TenuringTracer& mover) const { NativeObject* obj = object(); + if(!IsCellPointerValid(obj)) + return; // Beware JSObject::swap exchanging a native object for a non-native one. if (!obj->isNative()) @@ -2336,6 +2338,8 @@ js::gc::StoreBuffer::traceWholeCells(TenuringTracer& mover) { for (ArenaCellSet* cells = bufferWholeCell; cells; cells = cells->next) { Arena* arena = cells->arena; + if(!IsCellPointerValid(arena)) + continue; MOZ_ASSERT(arena->bufferedCells == cells); arena->bufferedCells = &ArenaCellSet::Empty; @@ -2364,6 +2368,7 @@ js::gc::StoreBuffer::CellPtrEdge::trace(TenuringTracer& mover) const { if (!*edge) return; + // XXX: We should check if the cell pointer is valid here too MOZ_ASSERT((*edge)->getTraceKind() == JS::TraceKind::Object); mover.traverse(reinterpret_cast(edge)); From ee8c57f6da5ab5de8e843bcb3a655f12801c057e Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Mon, 3 Aug 2020 15:07:49 -0400 Subject: [PATCH 08/16] Issue #1625 - Allow MailNews Oauth2 support to be configured in confvars.sh Baby has his bottle, you ok now? --- old-configure.in | 1 - 1 file changed, 1 deletion(-) diff --git a/old-configure.in b/old-configure.in index c2be28f9e5..8892c84607 100644 --- a/old-configure.in +++ b/old-configure.in @@ -2765,7 +2765,6 @@ MOZ_ARG_ENABLE_BOOL(mailnews, MOZ_MAILNEWS=) if test -n "$MOZ_MAILNEWS"; then - MOZ_MAILNEWS_OAUTH2=1 MOZ_MORK=1 MOZ_LDAP_XPCOM=1 From 5260565a0908fce2aa95928fcef9ac7d2f025955 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 17:13:33 -0400 Subject: [PATCH 09/16] Issue #1621 - Part 1: CSSEditUtils should use atom for CSS property if possible. There is a lot of string compare when using CSS property name. We should use nsGkAtoms instead. Ref: Bug 1323138 --- editor/libeditor/CSSEditUtils.cpp | 259 ++++++++++++++------------- editor/libeditor/CSSEditUtils.h | 65 ++++--- editor/libeditor/HTMLEditRules.cpp | 13 +- editor/libeditor/HTMLEditor.cpp | 53 +++--- editor/libeditor/HTMLStyleEditor.cpp | 22 +-- 5 files changed, 224 insertions(+), 188 deletions(-) diff --git a/editor/libeditor/CSSEditUtils.cpp b/editor/libeditor/CSSEditUtils.cpp index dd15a8730a..e15bc278fc 100644 --- a/editor/libeditor/CSSEditUtils.cpp +++ b/editor/libeditor/CSSEditUtils.cpp @@ -322,6 +322,15 @@ bool CSSEditUtils::IsCSSEditableProperty(nsINode* aNode, nsIAtom* aProperty, const nsAString* aAttribute) +{ + nsCOMPtr attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr; + return IsCSSEditableProperty(aNode, aProperty, attribute); +} + +bool +CSSEditUtils::IsCSSEditableProperty(nsINode* aNode, + nsIAtom* aProperty, + nsIAtom* aAttribute) { MOZ_ASSERT(aNode); @@ -339,13 +348,12 @@ CSSEditUtils::IsCSSEditableProperty(nsINode* aNode, nsGkAtoms::u == aProperty || nsGkAtoms::strike == aProperty || (nsGkAtoms::font == aProperty && aAttribute && - (aAttribute->EqualsLiteral("color") || - aAttribute->EqualsLiteral("face")))) { + (aAttribute == nsGkAtoms::color || aAttribute == nsGkAtoms::face))) { return true; } // ALIGN attribute on elements supporting it - if (aAttribute && (aAttribute->EqualsLiteral("align")) && + if (aAttribute == nsGkAtoms::align && node->IsAnyOfHTMLElements(nsGkAtoms::div, nsGkAtoms::p, nsGkAtoms::h1, @@ -368,7 +376,7 @@ CSSEditUtils::IsCSSEditableProperty(nsINode* aNode, return true; } - if (aAttribute && (aAttribute->EqualsLiteral("valign")) && + if (aAttribute == nsGkAtoms::valign && node->IsAnyOfHTMLElements(nsGkAtoms::col, nsGkAtoms::colgroup, nsGkAtoms::tbody, @@ -381,59 +389,52 @@ CSSEditUtils::IsCSSEditableProperty(nsINode* aNode, } // attributes TEXT, BACKGROUND and BGCOLOR on BODY - if (aAttribute && node->IsHTMLElement(nsGkAtoms::body) && - (aAttribute->EqualsLiteral("text") - || aAttribute->EqualsLiteral("background") - || aAttribute->EqualsLiteral("bgcolor"))) { + if (node->IsHTMLElement(nsGkAtoms::body) && + (aAttribute == nsGkAtoms::text || aAttribute == nsGkAtoms::background || + aAttribute == nsGkAtoms::bgcolor)) { return true; } // attribute BGCOLOR on other elements - if (aAttribute && aAttribute->EqualsLiteral("bgcolor")) { + if (aAttribute == nsGkAtoms::bgcolor) { return true; } // attributes HEIGHT, WIDTH and NOWRAP on TD and TH - if (aAttribute && - node->IsAnyOfHTMLElements(nsGkAtoms::td, nsGkAtoms::th) && - (aAttribute->EqualsLiteral("height") - || aAttribute->EqualsLiteral("width") - || aAttribute->EqualsLiteral("nowrap"))) { + if (node->IsAnyOfHTMLElements(nsGkAtoms::td, nsGkAtoms::th) && + (aAttribute == nsGkAtoms::height || aAttribute == nsGkAtoms::width || + aAttribute == nsGkAtoms::nowrap)) { return true; } // attributes HEIGHT and WIDTH on TABLE - if (aAttribute && node->IsHTMLElement(nsGkAtoms::table) && - (aAttribute->EqualsLiteral("height") - || aAttribute->EqualsLiteral("width"))) { + if (node->IsHTMLElement(nsGkAtoms::table) && + (aAttribute == nsGkAtoms::height || aAttribute == nsGkAtoms::width)) { return true; } // attributes SIZE and WIDTH on HR - if (aAttribute && node->IsHTMLElement(nsGkAtoms::hr) && - (aAttribute->EqualsLiteral("size") - || aAttribute->EqualsLiteral("width"))) { + if (node->IsHTMLElement(nsGkAtoms::hr) && + (aAttribute == nsGkAtoms::size || aAttribute == nsGkAtoms::width)) { return true; } // attribute TYPE on OL UL LI - if (aAttribute && - node->IsAnyOfHTMLElements(nsGkAtoms::ol, nsGkAtoms::ul, + if (node->IsAnyOfHTMLElements(nsGkAtoms::ol, nsGkAtoms::ul, nsGkAtoms::li) && - aAttribute->EqualsLiteral("type")) { + aAttribute == nsGkAtoms::type) { return true; } - if (aAttribute && node->IsHTMLElement(nsGkAtoms::img) && - (aAttribute->EqualsLiteral("border") - || aAttribute->EqualsLiteral("width") - || aAttribute->EqualsLiteral("height"))) { + if (node->IsHTMLElement(nsGkAtoms::img) && + (aAttribute == nsGkAtoms::border || aAttribute == nsGkAtoms::width || + aAttribute == nsGkAtoms::height)) { return true; } // other elements that we can align using CSS even if they // can't carry the html ALIGN attribute - if (aAttribute && aAttribute->EqualsLiteral("align") && + if (aAttribute == nsGkAtoms::align && node->IsAnyOfHTMLElements(nsGkAtoms::ul, nsGkAtoms::ol, nsGkAtoms::dl, @@ -818,7 +819,7 @@ void CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle( Element* aElement, nsIAtom* aHTMLProperty, - const nsAString* aAttribute, + nsIAtom* aAttribute, const nsAString* aValue, nsTArray& cssPropertyArray, nsTArray& cssValueArray, @@ -838,21 +839,20 @@ CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle( } else if (nsGkAtoms::tt == aHTMLProperty) { equivTable = ttEquivTable; } else if (aAttribute) { - if (nsGkAtoms::font == aHTMLProperty && - aAttribute->EqualsLiteral("color")) { + if (nsGkAtoms::font == aHTMLProperty && aAttribute == nsGkAtoms::color) { equivTable = fontColorEquivTable; } else if (nsGkAtoms::font == aHTMLProperty && - aAttribute->EqualsLiteral("face")) { + aAttribute == nsGkAtoms::face) { equivTable = fontFaceEquivTable; - } else if (aAttribute->EqualsLiteral("bgcolor")) { + } else if (aAttribute == nsGkAtoms::bgcolor) { equivTable = bgcolorEquivTable; - } else if (aAttribute->EqualsLiteral("background")) { + } else if (aAttribute == nsGkAtoms::background) { equivTable = backgroundImageEquivTable; - } else if (aAttribute->EqualsLiteral("text")) { + } else if (aAttribute == nsGkAtoms::text) { equivTable = textColorEquivTable; - } else if (aAttribute->EqualsLiteral("border")) { + } else if (aAttribute == nsGkAtoms::border) { equivTable = borderEquivTable; - } else if (aAttribute->EqualsLiteral("align")) { + } else if (aAttribute == nsGkAtoms::align) { if (aElement->IsHTMLElement(nsGkAtoms::table)) { equivTable = tableAlignEquivTable; } else if (aElement->IsHTMLElement(nsGkAtoms::hr)) { @@ -863,17 +863,17 @@ CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle( } else { equivTable = textAlignEquivTable; } - } else if (aAttribute->EqualsLiteral("valign")) { + } else if (aAttribute == nsGkAtoms::valign) { equivTable = verticalAlignEquivTable; - } else if (aAttribute->EqualsLiteral("nowrap")) { + } else if (aAttribute == nsGkAtoms::nowrap) { equivTable = nowrapEquivTable; - } else if (aAttribute->EqualsLiteral("width")) { + } else if (aAttribute == nsGkAtoms::width) { equivTable = widthEquivTable; - } else if (aAttribute->EqualsLiteral("height") || + } else if (aAttribute == nsGkAtoms::height || (aElement->IsHTMLElement(nsGkAtoms::hr) && - aAttribute->EqualsLiteral("size"))) { + aAttribute == nsGkAtoms::size)) { equivTable = heightEquivTable; - } else if (aAttribute->EqualsLiteral("type") && + } else if (aAttribute == nsGkAtoms::type && aElement->IsAnyOfHTMLElements(nsGkAtoms::ol, nsGkAtoms::ul, nsGkAtoms::li)) { @@ -890,40 +890,46 @@ CSSEditUtils::GenerateCSSDeclarationsFromHTMLStyle( // aValue for the node, and return in aCount the number of CSS properties set // by the call. The Element version returns aCount instead. int32_t -CSSEditUtils::SetCSSEquivalentToHTMLStyle(Element* aElement, +CSSEditUtils::SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode, nsIAtom* aProperty, const nsAString* aAttribute, const nsAString* aValue, bool aSuppressTransaction) { - MOZ_ASSERT(aElement && aProperty); MOZ_ASSERT_IF(aAttribute, aValue); - int32_t count; // This can only fail if SetCSSProperty fails, which should only happen if // something is pretty badly wrong. In this case we assert so that hopefully // someone will notice, but there's nothing more sensible to do than just // return the count and carry on. - nsresult rv = SetCSSEquivalentToHTMLStyle(aElement->AsDOMNode(), - aProperty, aAttribute, - aValue, &count, - aSuppressTransaction); - NS_ASSERTION(NS_SUCCEEDED(rv), "SetCSSEquivalentToHTMLStyle failed"); - NS_ENSURE_SUCCESS(rv, count); - return count; + nsCOMPtr element = do_QueryInterface(aNode); + return SetCSSEquivalentToHTMLStyle(element, + aProperty, aAttribute, + aValue, aSuppressTransaction); } -nsresult -CSSEditUtils::SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode, +int32_t +CSSEditUtils::SetCSSEquivalentToHTMLStyle(Element* aElement, nsIAtom* aHTMLProperty, const nsAString* aAttribute, const nsAString* aValue, - int32_t* aCount, bool aSuppressTransaction) { - nsCOMPtr element = do_QueryInterface(aNode); - *aCount = 0; - if (!element || !IsCSSEditableProperty(element, aHTMLProperty, aAttribute)) { - return NS_OK; + nsCOMPtr attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr; + return SetCSSEquivalentToHTMLStyle(aElement, aHTMLProperty, attribute, + aValue, aSuppressTransaction); +} + +int32_t +CSSEditUtils::SetCSSEquivalentToHTMLStyle(Element* aElement, + nsIAtom* aHTMLProperty, + nsIAtom* aAttribute, + const nsAString* aValue, + bool aSuppressTransaction) +{ + MOZ_ASSERT(aElement); + + if (!IsCSSEditableProperty(aElement, aHTMLProperty, aAttribute)) { + return 0; } // we can apply the styles only if the node is an element and if we have @@ -932,18 +938,20 @@ CSSEditUtils::SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode, // Find the CSS equivalence to the HTML style nsTArray cssPropertyArray; nsTArray cssValueArray; - GenerateCSSDeclarationsFromHTMLStyle(element, aHTMLProperty, aAttribute, + GenerateCSSDeclarationsFromHTMLStyle(aElement, aHTMLProperty, aAttribute, aValue, cssPropertyArray, cssValueArray, false); // set the individual CSS inline styles - *aCount = cssPropertyArray.Length(); - for (int32_t index = 0; index < *aCount; index++) { - nsresult rv = SetCSSProperty(*element, *cssPropertyArray[index], + size_t count = cssPropertyArray.Length(); + for (size_t index = 0; index < count; index++) { + nsresult rv = SetCSSProperty(*aElement, *cssPropertyArray[index], cssValueArray[index], aSuppressTransaction); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return 0; + } } - return NS_OK; + return count; } // Remove from aNode the CSS inline style equivalent to HTMLProperty/aAttribute/aValue for the node @@ -955,20 +963,22 @@ CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(nsIDOMNode* aNode, bool aSuppressTransaction) { nsCOMPtr element = do_QueryInterface(aNode); - NS_ENSURE_TRUE(element, NS_OK); + nsCOMPtr attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr; - return RemoveCSSEquivalentToHTMLStyle(element, aHTMLProperty, aAttribute, + return RemoveCSSEquivalentToHTMLStyle(element, aHTMLProperty, attribute, aValue, aSuppressTransaction); } nsresult CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(Element* aElement, nsIAtom* aHTMLProperty, - const nsAString* aAttribute, + nsIAtom* aAttribute, const nsAString* aValue, bool aSuppressTransaction) { - MOZ_ASSERT(aElement); + if (NS_WARN_IF(!aElement)) { + return NS_OK; + } if (!IsCSSEditableProperty(aElement, aHTMLProperty, aAttribute)) { return NS_OK; @@ -1003,7 +1013,7 @@ CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(Element* aElement, nsresult CSSEditUtils::GetCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode, nsIAtom* aHTMLProperty, - const nsAString* aAttribute, + nsIAtom* aAttribute, nsAString& aValueString, StyleType aStyleType) { @@ -1020,7 +1030,8 @@ CSSEditUtils::GetCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode, nsTArray cssValueArray; // get the CSS equivalence with last param true indicating we want only the // "gettable" properties - GenerateCSSDeclarationsFromHTMLStyle(theElement, aHTMLProperty, aAttribute, nullptr, + GenerateCSSDeclarationsFromHTMLStyle(theElement, aHTMLProperty, aAttribute, + nullptr, cssPropertyArray, cssValueArray, true); int32_t count = cssPropertyArray.Length(); for (int32_t index = 0; index < count; index++) { @@ -1066,48 +1077,58 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode, StyleType aStyleType) { MOZ_ASSERT(aNode && aProperty); - bool isSet; - nsresult rv = IsCSSEquivalentToHTMLInlineStyleSet(aNode->AsDOMNode(), - aProperty, aAttribute, - isSet, aValue, aStyleType); - NS_ENSURE_SUCCESS(rv, false); - return isSet; + nsCOMPtr attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr; + return IsCSSEquivalentToHTMLInlineStyleSet(aNode, + aProperty, attribute, + aValue, aStyleType); } -nsresult +bool +CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(nsIDOMNode* aNode, + nsIAtom* aProperty, + const nsAString* aAttribute, + nsAString& aValue, + StyleType aStyleType) +{ + MOZ_ASSERT(aNode && aProperty); + nsCOMPtr node = do_QueryInterface(aNode); + nsCOMPtr attribute = aAttribute ? NS_Atomize(*aAttribute) : nullptr; + return IsCSSEquivalentToHTMLInlineStyleSet(node, aProperty, attribute, + aValue, aStyleType); +} + +bool CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet( - nsIDOMNode* aNode, + nsINode* aNode, nsIAtom* aHTMLProperty, - const nsAString* aHTMLAttribute, - bool& aIsSet, + nsIAtom* aHTMLAttribute, nsAString& valueString, StyleType aStyleType) { - NS_ENSURE_TRUE(aNode, NS_ERROR_NULL_POINTER); + NS_ENSURE_TRUE(aNode, false); nsAutoString htmlValueString(valueString); - aIsSet = false; - nsCOMPtr node = do_QueryInterface(aNode); + bool isSet = false; do { valueString.Assign(htmlValueString); // get the value of the CSS equivalent styles nsresult rv = - GetCSSEquivalentToHTMLInlineStyleSet(node, aHTMLProperty, aHTMLAttribute, + GetCSSEquivalentToHTMLInlineStyleSet(aNode, aHTMLProperty, aHTMLAttribute, valueString, aStyleType); - NS_ENSURE_SUCCESS(rv, rv); + NS_ENSURE_SUCCESS(rv, false); // early way out if we can if (valueString.IsEmpty()) { - return NS_OK; + return isSet; } if (nsGkAtoms::b == aHTMLProperty) { if (valueString.EqualsLiteral("bold")) { - aIsSet = true; + isSet = true; } else if (valueString.EqualsLiteral("normal")) { - aIsSet = false; + isSet = false; } else if (valueString.EqualsLiteral("bolder")) { - aIsSet = true; + isSet = true; valueString.AssignLiteral("bold"); } else { int32_t weight = 0; @@ -1115,32 +1136,31 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet( nsAutoString value(valueString); weight = value.ToInteger(&errorCode); if (400 < weight) { - aIsSet = true; + isSet = true; valueString.AssignLiteral("bold"); } else { - aIsSet = false; + isSet = false; valueString.AssignLiteral("normal"); } } } else if (nsGkAtoms::i == aHTMLProperty) { if (valueString.EqualsLiteral("italic") || valueString.EqualsLiteral("oblique")) { - aIsSet = true; + isSet = true; } } else if (nsGkAtoms::u == aHTMLProperty) { nsAutoString val; val.AssignLiteral("underline"); - aIsSet = ChangeStyleTransaction::ValueIncludes(valueString, val); + isSet = ChangeStyleTransaction::ValueIncludes(valueString, val); } else if (nsGkAtoms::strike == aHTMLProperty) { nsAutoString val; val.AssignLiteral("line-through"); - aIsSet = ChangeStyleTransaction::ValueIncludes(valueString, val); - } else if (aHTMLAttribute && - ((nsGkAtoms::font == aHTMLProperty && - aHTMLAttribute->EqualsLiteral("color")) || - aHTMLAttribute->EqualsLiteral("bgcolor"))) { + isSet = ChangeStyleTransaction::ValueIncludes(valueString, val); + } else if ((nsGkAtoms::font == aHTMLProperty && + aHTMLAttribute == nsGkAtoms::color) || + aHTMLAttribute == nsGkAtoms::bgcolor) { if (htmlValueString.IsEmpty()) { - aIsSet = true; + isSet = true; } else { nscolor rgba; nsAutoString subStr; @@ -1174,54 +1194,53 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet( htmlColor.Append(char16_t(')')); } - aIsSet = htmlColor.Equals(valueString, - nsCaseInsensitiveStringComparator()); + isSet = htmlColor.Equals(valueString, + nsCaseInsensitiveStringComparator()); } else { - aIsSet = htmlValueString.Equals(valueString, - nsCaseInsensitiveStringComparator()); + isSet = htmlValueString.Equals(valueString, + nsCaseInsensitiveStringComparator()); } } } else if (nsGkAtoms::tt == aHTMLProperty) { - aIsSet = StringBeginsWith(valueString, NS_LITERAL_STRING("monospace")); + isSet = StringBeginsWith(valueString, NS_LITERAL_STRING("monospace")); } else if (nsGkAtoms::font == aHTMLProperty && aHTMLAttribute && - aHTMLAttribute->EqualsLiteral("face")) { + aHTMLAttribute == nsGkAtoms::face) { if (!htmlValueString.IsEmpty()) { const char16_t commaSpace[] = { char16_t(','), char16_t(' '), 0 }; const char16_t comma[] = { char16_t(','), 0 }; htmlValueString.ReplaceSubstring(commaSpace, comma); nsAutoString valueStringNorm(valueString); valueStringNorm.ReplaceSubstring(commaSpace, comma); - aIsSet = htmlValueString.Equals(valueStringNorm, - nsCaseInsensitiveStringComparator()); + isSet = htmlValueString.Equals(valueStringNorm, + nsCaseInsensitiveStringComparator()); } else { - aIsSet = true; + isSet = true; } - return NS_OK; - } else if (aHTMLAttribute && aHTMLAttribute->EqualsLiteral("align")) { - aIsSet = true; + return isSet; + } else if (aHTMLAttribute == nsGkAtoms::align) { + isSet = true; } else { - aIsSet = false; - return NS_OK; + return false; } if (!htmlValueString.IsEmpty() && htmlValueString.Equals(valueString, nsCaseInsensitiveStringComparator())) { - aIsSet = true; + isSet = true; } if (htmlValueString.EqualsLiteral("-moz-editor-invert-value")) { - aIsSet = !aIsSet; + isSet = !isSet; } if (nsGkAtoms::u == aHTMLProperty || nsGkAtoms::strike == aHTMLProperty) { // unfortunately, the value of the text-decoration property is not inherited. // that means that we have to look at ancestors of node to see if they are underlined - node = node->GetParentElement(); // set to null if it's not a dom element + aNode = aNode->GetParentElement(); // set to null if it's not a dom element } } while ((nsGkAtoms::u == aHTMLProperty || - nsGkAtoms::strike == aHTMLProperty) && !aIsSet && node); - return NS_OK; + nsGkAtoms::strike == aHTMLProperty) && !isSet && aNode); + return isSet; } void diff --git a/editor/libeditor/CSSEditUtils.h b/editor/libeditor/CSSEditUtils.h index 0b9a129527..5129ab88dd 100644 --- a/editor/libeditor/CSSEditUtils.h +++ b/editor/libeditor/CSSEditUtils.h @@ -90,6 +90,8 @@ public: */ bool IsCSSEditableProperty(nsINode* aNode, nsIAtom* aProperty, const nsAString* aAttribute); + bool IsCSSEditableProperty(nsINode* aNode, nsIAtom* aProperty, + nsIAtom* aAttribute); /** * Adds/remove a CSS declaration to the STYLE atrribute carried by a given @@ -188,14 +190,14 @@ public: * * @param aNode [IN] A DOM node. * @param aHTMLProperty [IN] An atom containing an HTML property. - * @param aAttribute [IN] A pointer to an attribute name or nullptr if + * @param aAttribute [IN] An atom of attribute name or nullptr if * irrelevant. * @param aValueString [OUT] The list of CSS values. * @param aStyleType [IN] eSpecified or eComputed. */ nsresult GetCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode, nsIAtom* aHTMLProperty, - const nsAString* aAttribute, + nsIAtom* aAttribute, nsAString& aValueString, StyleType aStyleType); @@ -205,16 +207,20 @@ public: * * @param aNode [IN] A DOM node. * @param aHTMLProperty [IN] An atom containing an HTML property. - * @param aAttribute [IN] A pointer to an attribute name or nullptr if - * irrelevant. - * @param aIsSet [OUT] A boolean being true if the css properties are - * set. + * @param aAttribute [IN] A pointer/atom to an attribute name or nullptr + * if irrelevant. * @param aValueString [IN/OUT] The attribute value (in) the list of CSS * values (out). * @param aStyleType [IN] eSpecified or eComputed. - * - * The nsIContent variant returns aIsSet instead of using an out parameter. + * @return A boolean being true if the css properties are + * set. */ + bool IsCSSEquivalentToHTMLInlineStyleSet(nsINode* aContent, + nsIAtom* aProperty, + nsIAtom* aAttribute, + nsAString& aValue, + StyleType aStyleType); + bool IsCSSEquivalentToHTMLInlineStyleSet(nsINode* aContent, nsIAtom* aProperty, const nsAString* aAttribute, @@ -227,12 +233,11 @@ public: nsAString& aValue, StyleType aStyleType); - nsresult IsCSSEquivalentToHTMLInlineStyleSet(nsIDOMNode* aNode, - nsIAtom* aHTMLProperty, - const nsAString* aAttribute, - bool& aIsSet, - nsAString& aValueString, - StyleType aStyleType); + bool IsCSSEquivalentToHTMLInlineStyleSet(nsIDOMNode* aNode, + nsIAtom* aProperty, + const nsAString* aAttribute, + nsAString& aValue, + StyleType aStyleType); /** * Adds to the node the CSS inline styles equivalent to the HTML style @@ -240,27 +245,29 @@ public: * * @param aNode [IN] A DOM node. * @param aHTMLProperty [IN] An atom containing an HTML property. - * @param aAttribute [IN] A pointer to an attribute name or nullptr if - * irrelevant. + * @param aAttribute [IN] A pointer/atom to an attribute name or nullptr + * if irrelevant. * @param aValue [IN] The attribute value. - * @param aCount [OUT] The number of CSS properties set by the call. * @param aSuppressTransaction [IN] A boolean indicating, when true, * that no transaction should be recorded. * - * aCount is returned by the dom::Element variant instead of being an out - * parameter. + * @return The number of CSS properties set by the call. */ + int32_t SetCSSEquivalentToHTMLStyle(dom::Element* aElement, + nsIAtom* aProperty, + nsIAtom* aAttribute, + const nsAString* aValue, + bool aSuppressTransaction); int32_t SetCSSEquivalentToHTMLStyle(dom::Element* aElement, nsIAtom* aProperty, const nsAString* aAttribute, const nsAString* aValue, bool aSuppressTransaction); - nsresult SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode, - nsIAtom* aHTMLProperty, - const nsAString* aAttribute, - const nsAString* aValue, - int32_t* aCount, - bool aSuppressTransaction); + int32_t SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode, + nsIAtom* aHTMLProperty, + const nsAString* aAttribute, + const nsAString* aValue, + bool aSuppressTransaction); /** * Removes from the node the CSS inline styles equivalent to the HTML style. @@ -284,7 +291,7 @@ public: * * @param aElement [IN] A DOM Element (must not be null). * @param aHTMLProperty [IN] An atom containing an HTML property. - * @param aAttribute [IN] A pointer to an attribute name or nullptr if + * @param aAttribute [IN] An atom to an attribute name or nullptr if * irrelevant. * @param aValue [IN] The attribute value. * @param aSuppressTransaction [IN] A boolean indicating, when true, @@ -292,7 +299,7 @@ public: */ nsresult RemoveCSSEquivalentToHTMLStyle(dom::Element* aElement, nsIAtom* aHTMLProperty, - const nsAString* aAttribute, + nsIAtom* aAttribute, const nsAString* aValue, bool aSuppressTransaction); @@ -409,7 +416,7 @@ private: * * @param aNode [IN] The DOM node. * @param aHTMLProperty [IN] An atom containing an HTML property. - * @param aAttribute [IN] A pointer to an attribute name or nullptr + * @param aAttribute [IN] An atom to an attribute name or nullptr * if irrelevant * @param aValue [IN] The attribute value. * @param aPropertyArray [OUT] The array of CSS properties. @@ -422,7 +429,7 @@ private: */ void GenerateCSSDeclarationsFromHTMLStyle(dom::Element* aNode, nsIAtom* aHTMLProperty, - const nsAString* aAttribute, + nsIAtom* aAttribute, const nsAString* aValue, nsTArray& aPropertyArray, nsTArray& aValueArray, diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index af4a43ab9e..14f6cfe111 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -840,19 +840,18 @@ HTMLEditRules::GetAlignment(bool* aMixed, NS_ENSURE_TRUE(nodeToExamine, NS_ERROR_NULL_POINTER); - NS_NAMED_LITERAL_STRING(typeAttrName, "align"); nsCOMPtr blockParent = htmlEditor->GetBlock(*nodeToExamine); NS_ENSURE_TRUE(blockParent, NS_ERROR_FAILURE); if (htmlEditor->IsCSSEnabled() && htmlEditor->mCSSEditUtils->IsCSSEditableProperty(blockParent, nullptr, - &typeAttrName)) { + nsGkAtoms::align)) { // We are in CSS mode and we know how to align this element with CSS nsAutoString value; // Let's get the value(s) of text-align or margin-left/margin-right htmlEditor->mCSSEditUtils->GetCSSEquivalentToHTMLInlineStyleSet( - blockParent, nullptr, &typeAttrName, value, CSSEditUtils::eComputed); + blockParent, nullptr, nsGkAtoms::align, value, CSSEditUtils::eComputed); if (value.EqualsLiteral("center") || value.EqualsLiteral("-moz-center") || value.EqualsLiteral("auto auto")) { @@ -4756,7 +4755,7 @@ HTMLEditRules::WillAlign(Selection& aSelection, NS_ENSURE_SUCCESS(rv, rv); if (useCSS) { htmlEditor->mCSSEditUtils->SetCSSEquivalentToHTMLStyle( - curNode->AsElement(), nullptr, &NS_LITERAL_STRING("align"), + curNode->AsElement(), nullptr, nsGkAtoms::align, &aAlignType, false); curDiv = nullptr; continue; @@ -7143,9 +7142,9 @@ HTMLEditRules::CacheInlineStyles(nsIDOMNode* aNode) isSet, &outValue); } else { NS_ENSURE_STATE(mHTMLEditor); - mHTMLEditor->mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(aNode, - mCachedStyles[j].tag, &(mCachedStyles[j].attr), isSet, outValue, - CSSEditUtils::eComputed); + isSet = mHTMLEditor->mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet( + aNode, mCachedStyles[j].tag, &(mCachedStyles[j].attr), outValue, + CSSEditUtils::eComputed); } if (isSet) { mCachedStyles[j].mPresent = true; diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index c2f0bdc6dd..82ef92042f 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -4398,30 +4398,35 @@ HTMLEditor::SetAttributeOrEquivalent(nsIDOMElement* aElement, nsAutoScriptBlocker scriptBlocker; if (IsCSSEnabled() && mCSSEditUtils) { - int32_t count; - nsresult rv = - mCSSEditUtils->SetCSSEquivalentToHTMLStyle(aElement, nullptr, - &aAttribute, &aValue, - &count, + nsCOMPtr element = do_QueryInterface(aElement); + MOZ_ASSERT(element); + + nsCOMPtr attribute = NS_Atomize(aAttribute); + MOZ_ASSERT(attribute); + + int32_t count = + mCSSEditUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, + attribute, &aValue, aSuppressTransaction); - NS_ENSURE_SUCCESS(rv, rv); if (count) { // we found an equivalence ; let's remove the HTML attribute itself if it is set nsAutoString existingValue; bool wasSet = false; - rv = GetAttributeValue(aElement, aAttribute, existingValue, &wasSet); + nsresult rv = + GetAttributeValue(aElement, aAttribute, existingValue, &wasSet); NS_ENSURE_SUCCESS(rv, rv); if (!wasSet) { return NS_OK; } - return aSuppressTransaction ? aElement->RemoveAttribute(aAttribute) : - RemoveAttribute(aElement, aAttribute); + return aSuppressTransaction ? + element->UnsetAttr(kNameSpaceID_None, attribute, true) : + RemoveAttribute(aElement, aAttribute); } // count is an integer that represents the number of CSS declarations applied to the // element. If it is zero, we found no equivalence in this implementation for the // attribute - if (aAttribute.EqualsLiteral("style")) { + if (attribute == nsGkAtoms::style) { // if it is the style attribute, just add the new value to the existing style // attribute's value nsAutoString existingValue; @@ -4432,14 +4437,15 @@ HTMLEditor::SetAttributeOrEquivalent(nsIDOMElement* aElement, existingValue.Append(' '); existingValue.Append(aValue); return aSuppressTransaction ? - aElement->SetAttribute(aAttribute, existingValue) : + element->SetAttr(kNameSpaceID_None, attribute, existingValue, true) : SetAttribute(aElement, aAttribute, existingValue); } // we have no CSS equivalence for this attribute and it is not the style // attribute; let's set it the good'n'old HTML way - return aSuppressTransaction ? aElement->SetAttribute(aAttribute, aValue) : - SetAttribute(aElement, aAttribute, aValue); + return aSuppressTransaction ? + element->SetAttr(kNameSpaceID_None, attribute, aValue, true) : + SetAttribute(aElement, aAttribute, aValue); } // we are not in an HTML+CSS editor; let's set the attribute the HTML way @@ -4461,7 +4467,7 @@ HTMLEditor::RemoveAttributeOrEquivalent(nsIDOMElement* aElement, if (IsCSSEnabled() && mCSSEditUtils) { nsresult rv = mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle( - element, nullptr, &aAttribute, nullptr, aSuppressTransaction); + element, nullptr, attribute, nullptr, aSuppressTransaction); NS_ENSURE_SUCCESS(rv, rv); } @@ -4523,7 +4529,6 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor) NS_ENSURE_SUCCESS(rv, rv); if (!cancel && !handled) { // Loop through the ranges in the selection - NS_NAMED_LITERAL_STRING(bgcolor, "bgcolor"); for (uint32_t i = 0; i < selection->RangeCount(); i++) { RefPtr range = selection->GetRangeAt(i); NS_ENSURE_TRUE(range, NS_ERROR_FAILURE); @@ -4542,13 +4547,15 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor) if (blockParent && cachedBlockParent != blockParent) { cachedBlockParent = blockParent; mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr, - &bgcolor, &aColor, false); + nsGkAtoms::bgcolor, + &aColor, false); } } else if (startNode == endNode && startNode->IsHTMLElement(nsGkAtoms::body) && isCollapsed) { // No block in the document, let's apply the background to the body mCSSEditUtils->SetCSSEquivalentToHTMLStyle(startNode->AsElement(), - nullptr, &bgcolor, &aColor, + nullptr, nsGkAtoms::bgcolor, + &aColor, false); } else if (startNode == endNode && (endOffset - startOffset == 1 || (!startOffset && !endOffset))) { @@ -4559,7 +4566,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor) if (blockParent && cachedBlockParent != blockParent) { cachedBlockParent = blockParent; mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr, - &bgcolor, &aColor, false); + nsGkAtoms::bgcolor, + &aColor, false); } } else { // Not the easy case. Range not contained in single text node. There @@ -4602,7 +4610,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor) if (blockParent && cachedBlockParent != blockParent) { cachedBlockParent = blockParent; mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr, - &bgcolor, &aColor, + nsGkAtoms::bgcolor, + &aColor, false); } } @@ -4613,7 +4622,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor) if (blockParent && cachedBlockParent != blockParent) { cachedBlockParent = blockParent; mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr, - &bgcolor, &aColor, + nsGkAtoms::bgcolor, + &aColor, false); } } @@ -4627,7 +4637,8 @@ HTMLEditor::SetCSSBackgroundColor(const nsAString& aColor) if (blockParent && cachedBlockParent != blockParent) { cachedBlockParent = blockParent; mCSSEditUtils->SetCSSEquivalentToHTMLStyle(blockParent, nullptr, - &bgcolor, &aColor, + nsGkAtoms::bgcolor, + &aColor, false); } } diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp index 72eb1d6908..d3eabba1d3 100644 --- a/editor/libeditor/HTMLStyleEditor.cpp +++ b/editor/libeditor/HTMLStyleEditor.cpp @@ -429,7 +429,7 @@ HTMLEditor::SetInlinePropertyOnNodeImpl(nsIContent& aNode, mCSSEditUtils->IsCSSEditableProperty(&aNode, &aProperty, aAttribute)) || // bgcolor is always done using CSS - aAttribute->EqualsLiteral("bgcolor"); + attrAtom == nsGkAtoms::bgcolor; if (useCSS) { nsCOMPtr tmp; @@ -444,12 +444,9 @@ HTMLEditor::SetInlinePropertyOnNodeImpl(nsIContent& aNode, } // Add the CSS styles corresponding to the HTML style request - int32_t count; - nsresult rv = - mCSSEditUtils->SetCSSEquivalentToHTMLStyle(tmp->AsDOMNode(), - &aProperty, aAttribute, - &aValue, &count, false); - NS_ENSURE_SUCCESS(rv, rv); + mCSSEditUtils->SetCSSEquivalentToHTMLStyle(tmp, + &aProperty, attrAtom, + &aValue, false); return NS_OK; } @@ -576,8 +573,9 @@ HTMLEditor::SplitStyleAbovePoint(nsCOMPtr* aNode, // in this implementation for the node; let's check if it carries those // CSS styles nsAutoString firstValue; - mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(GetAsDOMNode(node), - aProperty, aAttribute, isSet, firstValue, CSSEditUtils::eSpecified); + isSet = mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet( + node, aProperty, aAttribute, firstValue, + CSSEditUtils::eSpecified); } if (// node is the correct inline prop (aProperty && node->IsHTMLElement(aProperty)) || @@ -796,15 +794,17 @@ HTMLEditor::RemoveStyleInside(nsIContent& aNode, // the HTML style defined by aProperty/aAttribute has a CSS equivalence in // this implementation for the node aNode; let's check if it carries those // css styles + nsCOMPtr attribute = + aAttribute ? NS_Atomize(*aAttribute) : nullptr; nsAutoString propertyValue; bool isSet = mCSSEditUtils->IsCSSEquivalentToHTMLInlineStyleSet(&aNode, - aProperty, aAttribute, propertyValue, CSSEditUtils::eSpecified); + aProperty, attribute, propertyValue, CSSEditUtils::eSpecified); if (isSet && aNode.IsElement()) { // yes, tmp has the corresponding css declarations in its style attribute // let's remove them mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle(aNode.AsElement(), aProperty, - aAttribute, + attribute, &propertyValue, false); // remove the node if it is a span or font, if its style attribute is From b6b2792a35445de9c8e96067ca1e6e3164e5779e Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 18:25:26 -0400 Subject: [PATCH 10/16] Issue #1621 - Part 2: Implement nsIAtom version of SetAttribute/RemoveAttribute/CloneAttirubte. Add nsIAtom version of the following. - CloneAttribute - RemoveAttribute - RemoveAttributeOrEquivalent - SetAttribute - SetAttributeOrEquivalent Ref: Bug 1324996 --- editor/libeditor/EditorBase.cpp | 81 ++++++++++++++++------- editor/libeditor/EditorBase.h | 13 ++++ editor/libeditor/HTMLEditor.cpp | 114 +++++++++++++++----------------- editor/libeditor/HTMLEditor.h | 18 ++--- editor/libeditor/TextEditor.cpp | 8 +-- editor/libeditor/TextEditor.h | 19 +++--- 6 files changed, 148 insertions(+), 105 deletions(-) diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 3f419a74ed..7212e7c93d 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -1235,12 +1235,23 @@ EditorBase::SetAttribute(nsIDOMElement* aElement, const nsAString& aAttribute, const nsAString& aValue) { + if (NS_WARN_IF(aAttribute.IsEmpty())) { + return NS_ERROR_FAILURE; + } nsCOMPtr element = do_QueryInterface(aElement); NS_ENSURE_TRUE(element, NS_ERROR_NULL_POINTER); nsCOMPtr attribute = NS_Atomize(aAttribute); + return SetAttribute(element, attribute, aValue); +} + +nsresult +EditorBase::SetAttribute(Element* aElement, + nsIAtom* aAttribute, + const nsAString& aValue) +{ RefPtr transaction = - CreateTxnForSetAttribute(*element, *attribute, aValue); + CreateTxnForSetAttribute(*aElement, *aAttribute, aValue); return DoTransaction(transaction); } @@ -1269,12 +1280,22 @@ NS_IMETHODIMP EditorBase::RemoveAttribute(nsIDOMElement* aElement, const nsAString& aAttribute) { + if (NS_WARN_IF(aAttribute.IsEmpty())) { + return NS_ERROR_FAILURE; + } nsCOMPtr element = do_QueryInterface(aElement); NS_ENSURE_TRUE(element, NS_ERROR_NULL_POINTER); nsCOMPtr attribute = NS_Atomize(aAttribute); + return RemoveAttribute(element, attribute); +} + +nsresult +EditorBase::RemoveAttribute(Element* aElement, + nsIAtom* aAttribute) +{ RefPtr transaction = - CreateTxnForRemoveAttribute(*element, *attribute); + CreateTxnForRemoveAttribute(*aElement, *aAttribute); return DoTransaction(transaction); } @@ -2249,25 +2270,28 @@ EditorBase::CloneAttribute(const nsAString& aAttribute, nsIDOMNode* aSourceNode) { NS_ENSURE_TRUE(aDestNode && aSourceNode, NS_ERROR_NULL_POINTER); - - nsCOMPtr destElement = do_QueryInterface(aDestNode); - nsCOMPtr sourceElement = do_QueryInterface(aSourceNode); - NS_ENSURE_TRUE(destElement && sourceElement, NS_ERROR_NO_INTERFACE); - - nsAutoString attrValue; - bool isAttrSet; - nsresult rv = GetAttributeValue(sourceElement, - aAttribute, - attrValue, - &isAttrSet); - NS_ENSURE_SUCCESS(rv, rv); - if (isAttrSet) { - rv = SetAttribute(destElement, aAttribute, attrValue); - } else { - rv = RemoveAttribute(destElement, aAttribute); + if (NS_WARN_IF(aAttribute.IsEmpty())) { + return NS_ERROR_FAILURE; } - return rv; + nsCOMPtr destElement = do_QueryInterface(aDestNode); + nsCOMPtr sourceElement = do_QueryInterface(aSourceNode); + NS_ENSURE_TRUE(destElement && sourceElement, NS_ERROR_NO_INTERFACE); + + nsCOMPtr attribute = NS_Atomize(aAttribute); + return CloneAttribute(attribute, destElement, sourceElement); +} + +nsresult +EditorBase::CloneAttribute(nsIAtom* aAttribute, + Element* aDestElement, + Element* aSourceElement) +{ + nsAutoString attrValue; + if (aSourceElement->GetAttr(kNameSpaceID_None, aAttribute, attrValue)) { + return SetAttribute(aDestElement, aAttribute, attrValue); + } + return RemoveAttribute(aDestElement, aAttribute); } /** @@ -4678,21 +4702,32 @@ EditorBase::CreateHTMLContent(nsIAtom* aTag) kNameSpaceID_XHTML); } -nsresult +NS_IMETHODIMP EditorBase::SetAttributeOrEquivalent(nsIDOMElement* aElement, const nsAString& aAttribute, const nsAString& aValue, bool aSuppressTransaction) { - return SetAttribute(aElement, aAttribute, aValue); + nsCOMPtr element = do_QueryInterface(aElement); + if (NS_WARN_IF(!element)) { + return NS_ERROR_NULL_POINTER; + } + nsCOMPtr attribute = NS_Atomize(aAttribute); + return SetAttributeOrEquivalent(element, attribute, aValue, + aSuppressTransaction); } -nsresult +NS_IMETHODIMP EditorBase::RemoveAttributeOrEquivalent(nsIDOMElement* aElement, const nsAString& aAttribute, bool aSuppressTransaction) { - return RemoveAttribute(aElement, aAttribute); + nsCOMPtr element = do_QueryInterface(aElement); + if (NS_WARN_IF(!element)) { + return NS_ERROR_NULL_POINTER; + } + nsCOMPtr attribute = NS_Atomize(aAttribute); + return RemoveAttributeOrEquivalent(element, attribute, aSuppressTransaction); } nsresult diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index 618da12a8f..08a895dcdc 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -291,6 +291,19 @@ public: nsresult JoinNodes(nsINode& aLeftNode, nsINode& aRightNode); nsresult MoveNode(nsIContent* aNode, nsINode* aParent, int32_t aOffset); + nsresult CloneAttribute(nsIAtom* aAttribute, Element* aDestElement, + Element* aSourceElement); + nsresult RemoveAttribute(Element* aElement, nsIAtom* aAttribute); + virtual nsresult RemoveAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, + bool aSuppressTransaction) = 0; + nsresult SetAttribute(Element* aElement, nsIAtom* aAttribute, + const nsAString& aValue); + virtual nsresult SetAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, + const nsAString& aValue, + bool aSuppressTransaction) = 0; + /** * Method to replace certain CreateElementNS() calls. * diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 82ef92042f..4cda49ab51 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -4390,93 +4390,83 @@ HTMLEditor::IsEmptyNodeImpl(nsINode* aNode, // add to aElement the CSS inline styles corresponding to the HTML attribute // aAttribute with its value aValue nsresult -HTMLEditor::SetAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, +HTMLEditor::SetAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, const nsAString& aValue, bool aSuppressTransaction) { + MOZ_ASSERT(aElement); + MOZ_ASSERT(aAttribute); + nsAutoScriptBlocker scriptBlocker; - if (IsCSSEnabled() && mCSSEditUtils) { - nsCOMPtr element = do_QueryInterface(aElement); - MOZ_ASSERT(element); - - nsCOMPtr attribute = NS_Atomize(aAttribute); - MOZ_ASSERT(attribute); - - int32_t count = - mCSSEditUtils->SetCSSEquivalentToHTMLStyle(element, nullptr, - attribute, &aValue, - aSuppressTransaction); - if (count) { - // we found an equivalence ; let's remove the HTML attribute itself if it is set - nsAutoString existingValue; - bool wasSet = false; - nsresult rv = - GetAttributeValue(aElement, aAttribute, existingValue, &wasSet); - NS_ENSURE_SUCCESS(rv, rv); - if (!wasSet) { - return NS_OK; - } - return aSuppressTransaction ? - element->UnsetAttr(kNameSpaceID_None, attribute, true) : - RemoveAttribute(aElement, aAttribute); - } - - // count is an integer that represents the number of CSS declarations applied to the - // element. If it is zero, we found no equivalence in this implementation for the - // attribute - if (attribute == nsGkAtoms::style) { - // if it is the style attribute, just add the new value to the existing style - // attribute's value - nsAutoString existingValue; - bool wasSet = false; - nsresult rv = GetAttributeValue(aElement, NS_LITERAL_STRING("style"), - existingValue, &wasSet); - NS_ENSURE_SUCCESS(rv, rv); - existingValue.Append(' '); - existingValue.Append(aValue); - return aSuppressTransaction ? - element->SetAttr(kNameSpaceID_None, attribute, existingValue, true) : - SetAttribute(aElement, aAttribute, existingValue); - } - - // we have no CSS equivalence for this attribute and it is not the style - // attribute; let's set it the good'n'old HTML way + if (!IsCSSEnabled() || !mCSSEditUtils) { + // we are not in an HTML+CSS editor; let's set the attribute the HTML way return aSuppressTransaction ? - element->SetAttr(kNameSpaceID_None, attribute, aValue, true) : + aElement->SetAttr(kNameSpaceID_None, aAttribute, aValue, true) : SetAttribute(aElement, aAttribute, aValue); } - // we are not in an HTML+CSS editor; let's set the attribute the HTML way - return aSuppressTransaction ? aElement->SetAttribute(aAttribute, aValue) : - SetAttribute(aElement, aAttribute, aValue); + int32_t count = + mCSSEditUtils->SetCSSEquivalentToHTMLStyle(aElement, nullptr, + aAttribute, &aValue, + aSuppressTransaction); + if (count) { + // we found an equivalence ; let's remove the HTML attribute itself if it + // is set + nsAutoString existingValue; + if (!aElement->GetAttr(kNameSpaceID_None, aAttribute, existingValue)) { + return NS_OK; + } + + return aSuppressTransaction ? + aElement->UnsetAttr(kNameSpaceID_None, aAttribute, true) : + RemoveAttribute(aElement, aAttribute); + } + + // count is an integer that represents the number of CSS declarations applied + // to the element. If it is zero, we found no equivalence in this + // implementation for the attribute + if (aAttribute == nsGkAtoms::style) { + // if it is the style attribute, just add the new value to the existing + // style attribute's value + nsAutoString existingValue; + aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::style, existingValue); + existingValue.Append(' '); + existingValue.Append(aValue); + return aSuppressTransaction ? + aElement->SetAttr(kNameSpaceID_None, aAttribute, existingValue, true) : + SetAttribute(aElement, aAttribute, existingValue); + } + + // we have no CSS equivalence for this attribute and it is not the style + // attribute; let's set it the good'n'old HTML way + return aSuppressTransaction ? + aElement->SetAttr(kNameSpaceID_None, aAttribute, aValue, true) : + SetAttribute(aElement, aAttribute, aValue); } nsresult -HTMLEditor::RemoveAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, +HTMLEditor::RemoveAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, bool aSuppressTransaction) { - nsCOMPtr element = do_QueryInterface(aElement); - NS_ENSURE_TRUE(element, NS_OK); - - nsCOMPtr attribute = NS_Atomize(aAttribute); - MOZ_ASSERT(attribute); + MOZ_ASSERT(aElement); + MOZ_ASSERT(aAttribute); if (IsCSSEnabled() && mCSSEditUtils) { nsresult rv = mCSSEditUtils->RemoveCSSEquivalentToHTMLStyle( - element, nullptr, attribute, nullptr, aSuppressTransaction); + aElement, nullptr, aAttribute, nullptr, aSuppressTransaction); NS_ENSURE_SUCCESS(rv, rv); } - if (!element->HasAttr(kNameSpaceID_None, attribute)) { + if (!aElement->HasAttr(kNameSpaceID_None, aAttribute)) { return NS_OK; } return aSuppressTransaction ? - element->UnsetAttr(kNameSpaceID_None, attribute, /* aNotify = */ true) : + aElement->UnsetAttr(kNameSpaceID_None, aAttribute, /* aNotify = */ true) : RemoveAttribute(aElement, aAttribute); } diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index dc1a41b70a..494e7c3834 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -118,6 +118,16 @@ public: virtual already_AddRefed GetInputEventTargetContent() override; virtual bool IsEditable(nsINode* aNode) override; using EditorBase::IsEditable; + virtual nsresult RemoveAttributeOrEquivalent( + Element* aElement, + nsIAtom* aAttribute, + bool aSuppressTransaction) override; + virtual nsresult SetAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, + const nsAString& aValue, + bool aSuppressTransaction) override; + using EditorBase::RemoveAttributeOrEquivalent; + using EditorBase::SetAttributeOrEquivalent; // nsStubMutationObserver overrides NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED @@ -329,14 +339,6 @@ public: */ virtual nsresult SelectEntireDocument(Selection* aSelection) override; - NS_IMETHOD SetAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, - const nsAString& aValue, - bool aSuppressTransaction) override; - NS_IMETHOD RemoveAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, - bool aSuppressTransaction) override; - /** * Join together any adjacent editable text nodes in the range. */ diff --git a/editor/libeditor/TextEditor.cpp b/editor/libeditor/TextEditor.cpp index 07b06a96aa..545e3a3d17 100644 --- a/editor/libeditor/TextEditor.cpp +++ b/editor/libeditor/TextEditor.cpp @@ -1622,8 +1622,8 @@ TextEditor::GetDOMEventTarget() nsresult -TextEditor::SetAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, +TextEditor::SetAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, const nsAString& aValue, bool aSuppressTransaction) { @@ -1631,8 +1631,8 @@ TextEditor::SetAttributeOrEquivalent(nsIDOMElement* aElement, } nsresult -TextEditor::RemoveAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, +TextEditor::RemoveAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, bool aSuppressTransaction) { return EditorBase::RemoveAttribute(aElement, aAttribute); diff --git a/editor/libeditor/TextEditor.h b/editor/libeditor/TextEditor.h index 31c551f852..7bb594931b 100644 --- a/editor/libeditor/TextEditor.h +++ b/editor/libeditor/TextEditor.h @@ -63,14 +63,17 @@ public: // nsIEditorMailSupport overrides NS_DECL_NSIEDITORMAILSUPPORT - // Overrides of EditorBase interface methods - NS_IMETHOD SetAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, - const nsAString& aValue, - bool aSuppressTransaction) override; - NS_IMETHOD RemoveAttributeOrEquivalent(nsIDOMElement* aElement, - const nsAString& aAttribute, - bool aSuppressTransaction) override; + // Overrides of EditorBase + virtual nsresult RemoveAttributeOrEquivalent( + Element* aElement, + nsIAtom* aAttribute, + bool aSuppressTransaction) override; + virtual nsresult SetAttributeOrEquivalent(Element* aElement, + nsIAtom* aAttribute, + const nsAString& aValue, + bool aSuppressTransaction) override; + using EditorBase::RemoveAttributeOrEquivalent; + using EditorBase::SetAttributeOrEquivalent; NS_IMETHOD Init(nsIDOMDocument* aDoc, nsIContent* aRoot, nsISelectionController* aSelCon, uint32_t aFlags, From ec1301a85dc265ded0e0c3fcea97441860ecd5cf Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 18:38:33 -0400 Subject: [PATCH 11/16] Issue #1621 - Part 3: Use nsIAtom to change attirbute if possible. We can replace old nsIEditor API with nsIAtom version. Ref: Bug 1324996 --- editor/libeditor/EditorBase.cpp | 14 +++--- editor/libeditor/HTMLEditRules.cpp | 51 +++++++++----------- editor/libeditor/HTMLEditor.cpp | 5 +- editor/libeditor/HTMLEditorObjectResizer.cpp | 29 +++++------ editor/libeditor/HTMLStyleEditor.cpp | 8 ++- editor/libeditor/TextEditRules.cpp | 4 +- editor/libeditor/TextEditor.cpp | 4 +- 7 files changed, 51 insertions(+), 64 deletions(-) diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 7212e7c93d..60df3571ef 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -2330,11 +2330,9 @@ EditorBase::CloneAttributes(Element* aDest, RefPtr destAttributes = aDest->Attributes(); while (RefPtr attr = destAttributes->Item(0)) { if (destInBody) { - RemoveAttribute(static_cast(GetAsDOMNode(aDest)), - attr->NodeName()); + RemoveAttribute(aDest, attr->NodeInfo()->NameAtom()); } else { - ErrorResult ignored; - aDest->RemoveAttribute(attr->NodeName(), ignored); + aDest->UnsetAttr(kNameSpaceID_None, attr->NodeInfo()->NameAtom(), true); } } @@ -2346,13 +2344,13 @@ EditorBase::CloneAttributes(Element* aDest, nsAutoString value; attr->GetValue(value); if (destInBody) { - SetAttributeOrEquivalent(static_cast(GetAsDOMNode(aDest)), - attr->NodeName(), value, false); + SetAttributeOrEquivalent(aDest, attr->NodeInfo()->NameAtom(), value, + false); } else { // The element is not inserted in the document yet, we don't want to put // a transaction on the UndoStack - SetAttributeOrEquivalent(static_cast(GetAsDOMNode(aDest)), - attr->NodeName(), value, true); + SetAttributeOrEquivalent(aDest, attr->NodeInfo()->NameAtom(), value, + true); } } } diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index 14f6cfe111..fe5e6c2bf5 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -3302,15 +3302,15 @@ HTMLEditRules::WillMakeList(Selection* aSelection, } } NS_ENSURE_STATE(mHTMLEditor); - nsCOMPtr curElement = do_QueryInterface(curNode); - NS_NAMED_LITERAL_STRING(typestr, "type"); + nsCOMPtr curElement = do_QueryInterface(curNode); if (aBulletType && !aBulletType->IsEmpty()) { - rv = mHTMLEditor->SetAttribute(curElement, typestr, *aBulletType); + rv = mHTMLEditor->SetAttribute(curElement, nsGkAtoms::type, + *aBulletType); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } else { - rv = mHTMLEditor->RemoveAttribute(curElement, typestr); + rv = mHTMLEditor->RemoveAttribute(curElement, nsGkAtoms::type); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -4836,7 +4836,6 @@ HTMLEditRules::AlignBlockContents(nsIDOMNode* aNode, nsCOMPtr node = do_QueryInterface(aNode); NS_ENSURE_TRUE(node && alignType, NS_ERROR_NULL_POINTER); nsCOMPtr firstChild, lastChild; - nsCOMPtr divNode; bool useCSS = mHTMLEditor->IsCSSEnabled(); @@ -4844,24 +4843,25 @@ HTMLEditRules::AlignBlockContents(nsIDOMNode* aNode, firstChild = mHTMLEditor->GetFirstEditableChild(*node); NS_ENSURE_STATE(mHTMLEditor); lastChild = mHTMLEditor->GetLastEditableChild(*node); - NS_NAMED_LITERAL_STRING(attr, "align"); if (!firstChild) { // this cell has no content, nothing to align } else if (firstChild == lastChild && firstChild->IsHTMLElement(nsGkAtoms::div)) { // the cell already has a div containing all of its content: just // act on this div. - nsCOMPtr divElem = do_QueryInterface(firstChild); + RefPtr divElem = firstChild->AsElement(); if (useCSS) { NS_ENSURE_STATE(mHTMLEditor); - nsresult rv = mHTMLEditor->SetAttributeOrEquivalent(divElem, attr, + nsresult rv = mHTMLEditor->SetAttributeOrEquivalent(divElem, + nsGkAtoms::align, *alignType, false); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } else { NS_ENSURE_STATE(mHTMLEditor); - nsresult rv = mHTMLEditor->SetAttribute(divElem, attr, *alignType); + nsresult rv = mHTMLEditor->SetAttribute(divElem, nsGkAtoms::align, + *alignType); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -4869,28 +4869,29 @@ HTMLEditRules::AlignBlockContents(nsIDOMNode* aNode, } else { // else we need to put in a div, set the alignment, and toss in all the children NS_ENSURE_STATE(mHTMLEditor); - divNode = mHTMLEditor->CreateNode(nsGkAtoms::div, node, 0); - NS_ENSURE_STATE(divNode); + RefPtr divElem = mHTMLEditor->CreateNode(nsGkAtoms::div, node, 0); + NS_ENSURE_STATE(divElem); // set up the alignment on the div - nsCOMPtr divElem = do_QueryInterface(divNode); if (useCSS) { NS_ENSURE_STATE(mHTMLEditor); nsresult rv = - mHTMLEditor->SetAttributeOrEquivalent(divElem, attr, *alignType, false); + mHTMLEditor->SetAttributeOrEquivalent(divElem, nsGkAtoms::align, + *alignType, false); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } else { NS_ENSURE_STATE(mHTMLEditor); - nsresult rv = mHTMLEditor->SetAttribute(divElem, attr, *alignType); + nsresult rv = + mHTMLEditor->SetAttribute(divElem, nsGkAtoms::align, *alignType); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } // tuck the children into the end of the active div - while (lastChild && (lastChild != divNode)) { + while (lastChild && (lastChild != divElem)) { NS_ENSURE_STATE(mHTMLEditor); - nsresult rv = mHTMLEditor->MoveNode(lastChild, divNode, 0); + nsresult rv = mHTMLEditor->MoveNode(lastChild, divElem, 0); NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_STATE(mHTMLEditor); lastChild = mHTMLEditor->GetLastEditableChild(*node); @@ -6510,9 +6511,9 @@ HTMLEditRules::SplitParagraph(nsIDOMNode *aPara, } // remove ID attribute on the paragraph we just created - nsCOMPtr rightElt = do_QueryInterface(rightPara); + RefPtr rightElt = rightPara->AsElement(); NS_ENSURE_STATE(mHTMLEditor); - rv = mHTMLEditor->RemoveAttribute(rightElt, NS_LITERAL_STRING("id")); + rv = mHTMLEditor->RemoveAttribute(rightElt, nsGkAtoms::id); NS_ENSURE_SUCCESS(rv, rv); // check both halves of para to see if we need mozBR @@ -8387,18 +8388,18 @@ HTMLEditRules::RemoveAlignment(nsIDOMNode* aNode, NS_ENSURE_SUCCESS(rv, rv); } else if (isBlock || HTMLEditUtils::IsHR(child)) { // the current node is a block element - nsCOMPtr curElem = do_QueryInterface(child); + nsCOMPtr curElem = do_QueryInterface(child); if (HTMLEditUtils::SupportsAlignAttr(child)) { // remove the ALIGN attribute if this element can have it NS_ENSURE_STATE(mHTMLEditor); - rv = mHTMLEditor->RemoveAttribute(curElem, NS_LITERAL_STRING("align")); + rv = mHTMLEditor->RemoveAttribute(curElem, nsGkAtoms::align); NS_ENSURE_SUCCESS(rv, rv); } if (useCSS) { if (HTMLEditUtils::IsTable(child) || HTMLEditUtils::IsHR(child)) { NS_ENSURE_STATE(mHTMLEditor); rv = mHTMLEditor->SetAttributeOrEquivalent(curElem, - NS_LITERAL_STRING("align"), + nsGkAtoms::align, aAlignType, false); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; @@ -8518,21 +8519,17 @@ HTMLEditRules::AlignBlock(Element& aElement, nsresult rv = RemoveAlignment(aElement.AsDOMNode(), aAlignType, aContentsOnly == ContentsOnly::yes); NS_ENSURE_SUCCESS(rv, rv); - NS_NAMED_LITERAL_STRING(attr, "align"); if (htmlEditor->IsCSSEnabled()) { // Let's use CSS alignment; we use margin-left and margin-right for tables // and text-align for other block-level elements rv = htmlEditor->SetAttributeOrEquivalent( - static_cast(aElement.AsDOMNode()), - attr, aAlignType, false); + &aElement, nsGkAtoms::align, aAlignType, false); NS_ENSURE_SUCCESS(rv, rv); } else { // HTML case; this code is supposed to be called ONLY if the element // supports the align attribute but we'll never know... if (HTMLEditUtils::SupportsAlignAttr(aElement.AsDOMNode())) { - rv = htmlEditor->SetAttribute( - static_cast(aElement.AsDOMNode()), - attr, aAlignType); + rv = htmlEditor->SetAttribute(&aElement, nsGkAtoms::align, aAlignType); NS_ENSURE_SUCCESS(rv, rv); } } diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 4cda49ab51..6a630cb1c4 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -2529,7 +2529,7 @@ HTMLEditor::CreateElementWithDefaults(const nsAString& aTagName) // New call to use instead to get proper HTML element, bug 39919 nsCOMPtr realTagAtom = NS_Atomize(realTagName); - nsCOMPtr newElement = CreateHTMLContent(realTagAtom); + RefPtr newElement = CreateHTMLContent(realTagAtom); if (!newElement) { return nullptr; } @@ -2561,8 +2561,7 @@ HTMLEditor::CreateElementWithDefaults(const nsAString& aTagName) } else if (tagName.EqualsLiteral("td")) { nsresult rv = SetAttributeOrEquivalent( - static_cast(newElement->AsDOMNode()), - NS_LITERAL_STRING("valign"), NS_LITERAL_STRING("top"), true); + newElement, nsGkAtoms::valign, NS_LITERAL_STRING("top"), true); NS_ENSURE_SUCCESS(rv, nullptr); } // ADD OTHER TAGS HERE diff --git a/editor/libeditor/HTMLEditorObjectResizer.cpp b/editor/libeditor/HTMLEditorObjectResizer.cpp index 111a3f9751..8ed5b6e4d0 100644 --- a/editor/libeditor/HTMLEditorObjectResizer.cpp +++ b/editor/libeditor/HTMLEditorObjectResizer.cpp @@ -926,35 +926,30 @@ HTMLEditor::SetFinalSize(int32_t aX, // we want one transaction only from a user's point of view AutoEditBatch batchIt(this); - NS_NAMED_LITERAL_STRING(widthStr, "width"); - NS_NAMED_LITERAL_STRING(heightStr, "height"); - - nsCOMPtr resizedObject = do_QueryInterface(mResizedObject); - NS_ENSURE_TRUE(resizedObject, ); if (mResizedObjectIsAbsolutelyPositioned) { if (setHeight) { - mCSSEditUtils->SetCSSPropertyPixels(*resizedObject, *nsGkAtoms::top, y); + mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::top, y); } if (setWidth) { - mCSSEditUtils->SetCSSPropertyPixels(*resizedObject, *nsGkAtoms::left, x); + mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::left, x); } } if (IsCSSEnabled() || mResizedObjectIsAbsolutelyPositioned) { if (setWidth && mResizedObject->HasAttr(kNameSpaceID_None, nsGkAtoms::width)) { - RemoveAttribute(static_cast(GetAsDOMNode(mResizedObject)), widthStr); + RemoveAttribute(mResizedObject, nsGkAtoms::width); } if (setHeight && mResizedObject->HasAttr(kNameSpaceID_None, nsGkAtoms::height)) { - RemoveAttribute(static_cast(GetAsDOMNode(mResizedObject)), heightStr); + RemoveAttribute(mResizedObject, nsGkAtoms::height); } if (setWidth) { - mCSSEditUtils->SetCSSPropertyPixels(*resizedObject, *nsGkAtoms::width, + mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::width, width); } if (setHeight) { - mCSSEditUtils->SetCSSPropertyPixels(*resizedObject, *nsGkAtoms::height, + mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::height, height); } } else { @@ -964,30 +959,30 @@ HTMLEditor::SetFinalSize(int32_t aX, // triggering an immediate reflow; otherwise, we have problems // with asynchronous reflow if (setWidth) { - mCSSEditUtils->SetCSSPropertyPixels(*resizedObject, *nsGkAtoms::width, + mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::width, width); } if (setHeight) { - mCSSEditUtils->SetCSSPropertyPixels(*resizedObject, *nsGkAtoms::height, + mCSSEditUtils->SetCSSPropertyPixels(*mResizedObject, *nsGkAtoms::height, height); } if (setWidth) { nsAutoString w; w.AppendInt(width); - SetAttribute(static_cast(GetAsDOMNode(mResizedObject)), widthStr, w); + SetAttribute(mResizedObject, nsGkAtoms::width, w); } if (setHeight) { nsAutoString h; h.AppendInt(height); - SetAttribute(static_cast(GetAsDOMNode(mResizedObject)), heightStr, h); + SetAttribute(mResizedObject, nsGkAtoms::height, h); } if (setWidth) { - mCSSEditUtils->RemoveCSSProperty(*resizedObject, *nsGkAtoms::width, + mCSSEditUtils->RemoveCSSProperty(*mResizedObject, *nsGkAtoms::width, EmptyString()); } if (setHeight) { - mCSSEditUtils->RemoveCSSProperty(*resizedObject, *nsGkAtoms::height, + mCSSEditUtils->RemoveCSSProperty(*mResizedObject, *nsGkAtoms::height, EmptyString()); } } diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp index d3eabba1d3..c41f0a05c7 100644 --- a/editor/libeditor/HTMLStyleEditor.cpp +++ b/editor/libeditor/HTMLStyleEditor.cpp @@ -744,8 +744,6 @@ HTMLEditor::RemoveStyleInside(nsIContent& aNode, // if we weren't passed an attribute, then we want to // remove any matching inlinestyles entirely if (!aAttribute || aAttribute->IsEmpty()) { - NS_NAMED_LITERAL_STRING(styleAttr, "style"); - NS_NAMED_LITERAL_STRING(classAttr, "class"); bool hasStyleAttr = aNode.HasAttr(kNameSpaceID_None, nsGkAtoms::style); bool hasClassAttr = aNode.HasAttr(kNameSpaceID_None, nsGkAtoms::_class); @@ -754,14 +752,14 @@ HTMLEditor::RemoveStyleInside(nsIContent& aNode, // just remove the element... We need to create above the element // a span that will carry those styles or class, then we can delete // the node. - nsCOMPtr spanNode = + RefPtr spanNode = InsertContainerAbove(&aNode, nsGkAtoms::span); NS_ENSURE_STATE(spanNode); nsresult rv = - CloneAttribute(styleAttr, spanNode->AsDOMNode(), aNode.AsDOMNode()); + CloneAttribute(nsGkAtoms::style, spanNode, aNode.AsElement()); NS_ENSURE_SUCCESS(rv, rv); rv = - CloneAttribute(classAttr, spanNode->AsDOMNode(), aNode.AsDOMNode()); + CloneAttribute(nsGkAtoms::_class, spanNode, aNode.AsElement()); NS_ENSURE_SUCCESS(rv, rv); } nsresult rv = RemoveContainer(&aNode); diff --git a/editor/libeditor/TextEditRules.cpp b/editor/libeditor/TextEditRules.cpp index 4ecaff722d..e98d36dd38 100644 --- a/editor/libeditor/TextEditRules.cpp +++ b/editor/libeditor/TextEditRules.cpp @@ -1422,9 +1422,9 @@ TextEditRules::CreateMozBR(nsIDOMNode* inParent, NS_ENSURE_SUCCESS(rv, rv); // give it special moz attr - nsCOMPtr brElem = do_QueryInterface(brNode); + nsCOMPtr brElem = do_QueryInterface(brNode); if (brElem) { - rv = mTextEditor->SetAttribute(brElem, NS_LITERAL_STRING("type"), + rv = mTextEditor->SetAttribute(brElem, nsGkAtoms::type, NS_LITERAL_STRING("_moz")); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/editor/libeditor/TextEditor.cpp b/editor/libeditor/TextEditor.cpp index 545e3a3d17..3bee7843ce 100644 --- a/editor/libeditor/TextEditor.cpp +++ b/editor/libeditor/TextEditor.cpp @@ -311,9 +311,9 @@ TextEditor::UpdateMetaCharset(nsIDOMDocument* aDocument, } // set attribute to charset=text/html - nsCOMPtr metaElement = do_QueryInterface(metaNode); + RefPtr metaElement = metaNode->AsElement(); MOZ_ASSERT(metaElement); - rv = EditorBase::SetAttribute(metaElement, NS_LITERAL_STRING("content"), + rv = EditorBase::SetAttribute(metaElement, nsGkAtoms::content, Substring(originalStart, start) + charsetEquals + NS_ConvertASCIItoUTF16(aCharacterSet)); From 57034543290a6dc6dd314736c259147fd616b973 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Fri, 22 May 2020 22:26:28 -0400 Subject: [PATCH 12/16] Issue #1621 - Part 4: Check whether node can be splited. At first, HTMLEditor::GetActiveEditingHost might return null in this situation, we should check whether nullptr is returned. At second, SplitNodeDeep returns error since curent is design mode and selection node has no parent. So we should check error. Ref: Bug 1350772 --- editor/libeditor/HTMLEditRules.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index fe5e6c2bf5..f24d0131d1 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -1509,10 +1509,11 @@ HTMLEditRules::WillInsertBreak(Selection& aSelection, nsCOMPtr blockParent = htmlEditor->GetBlock(node); NS_ENSURE_TRUE(blockParent, NS_ERROR_FAILURE); - // If the active editing host is an inline element, or if the active editing - // host is the block parent itself, just append a br. + // When there is an active editing host (the if it's in designMode) + // and a block which becomes the parent of line breaker is in it, do the + // standard thing. nsCOMPtr host = htmlEditor->GetActiveEditingHost(); - if (!EditorUtils::IsDescendantOf(blockParent, host)) { + if (host && !EditorUtils::IsDescendantOf(blockParent, host)) { nsresult rv = StandardBreakImpl(node, offset, aSelection); NS_ENSURE_SUCCESS(rv, rv); *aHandled = true; @@ -6498,10 +6499,14 @@ HTMLEditRules::SplitParagraph(nsIDOMNode *aPara, // split the paragraph NS_ENSURE_STATE(mHTMLEditor); NS_ENSURE_STATE(selNode->IsContent()); - mHTMLEditor->SplitNodeDeep(*para, *selNode->AsContent(), *aOffset, - HTMLEditor::EmptyContainers::yes, - getter_AddRefs(leftPara), - getter_AddRefs(rightPara)); + int32_t offset = + mHTMLEditor->SplitNodeDeep(*para, *selNode->AsContent(), *aOffset, + HTMLEditor::EmptyContainers::yes, + getter_AddRefs(leftPara), + getter_AddRefs(rightPara)); + if (NS_WARN_IF(offset == -1)) { + return NS_ERROR_FAILURE; + } // get rid of the break, if it is visible (otherwise it may be needed to prevent an empty p) NS_ENSURE_STATE(mHTMLEditor); if (mHTMLEditor->IsVisBreak(aBRNode)) { From 3bfc57422015a7f356b9b6a55f23561861b3553a Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 7 Aug 2020 14:11:20 -0400 Subject: [PATCH 13/16] Issue #1628 - Remove redundant PREF_EM_UPDATE_BACKGROUND_URL --- toolkit/mozapps/extensions/AddonManager.jsm | 1 - toolkit/mozapps/extensions/internal/XPIProvider.jsm | 8 +------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/toolkit/mozapps/extensions/AddonManager.jsm b/toolkit/mozapps/extensions/AddonManager.jsm index a48e0034fd..f69daa0eb9 100644 --- a/toolkit/mozapps/extensions/AddonManager.jsm +++ b/toolkit/mozapps/extensions/AddonManager.jsm @@ -30,7 +30,6 @@ const PREF_EM_LAST_PLATFORM_VERSION = "extensions.lastPlatformVersion"; const PREF_EM_AUTOUPDATE_DEFAULT = "extensions.update.autoUpdateDefault"; const PREF_EM_STRICT_COMPATIBILITY = "extensions.strictCompatibility"; const PREF_EM_CHECK_UPDATE_SECURITY = "extensions.checkUpdateSecurity"; -const PREF_EM_UPDATE_BACKGROUND_URL = "extensions.update.background.url"; const PREF_APP_UPDATE_ENABLED = "app.update.enabled"; const PREF_APP_UPDATE_AUTO = "app.update.auto"; const PREF_MATCH_OS_LOCALE = "intl.locale.matchOS"; diff --git a/toolkit/mozapps/extensions/internal/XPIProvider.jsm b/toolkit/mozapps/extensions/internal/XPIProvider.jsm index a8594036c9..e47599da1b 100644 --- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm +++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm @@ -74,7 +74,6 @@ const PREF_DSS_SWITCHPENDING = "extensions.dss.switchPending"; const PREF_DSS_SKIN_TO_SELECT = "extensions.lastSelectedSkin"; const PREF_GENERAL_SKINS_SELECTEDSKIN = "general.skins.selectedSkin"; const PREF_EM_UPDATE_URL = "extensions.update.url"; -const PREF_EM_UPDATE_BACKGROUND_URL = "extensions.update.background.url"; const PREF_EM_ENABLED_ADDONS = "extensions.enabledAddons"; const PREF_EM_EXTENSION_FORMAT = "extensions."; const PREF_EM_ENABLED_SCOPES = "extensions.enabledScopes"; @@ -6179,12 +6178,7 @@ function UpdateChecker(aAddon, aListener, aReason, aAppVersion, aPlatformVersion let updateURL = aAddon.updateURL; if (!updateURL) { - if (aReason == AddonManager.UPDATE_WHEN_PERIODIC_UPDATE && - Services.prefs.getPrefType(PREF_EM_UPDATE_BACKGROUND_URL) == Services.prefs.PREF_STRING) { - updateURL = Services.prefs.getCharPref(PREF_EM_UPDATE_BACKGROUND_URL); - } else { - updateURL = Services.prefs.getCharPref(PREF_EM_UPDATE_URL); - } + updateURL = Services.prefs.getCharPref(PREF_EM_UPDATE_URL); } const UPDATE_TYPE_COMPATIBILITY = 32; From 26990a1d74a328f1f1b99e5b11ef351fccb1d190 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 7 Aug 2020 21:15:52 +0000 Subject: [PATCH 14/16] Pref and disable getRootNode() This is apparently used for fallback selection and if available it is "assumed" Shadow DOM is also available, while this is a utility function. Webcompat is a nightmare sometimes. --- dom/webidl/Node.webidl | 2 +- modules/libpref/init/all.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/dom/webidl/Node.webidl b/dom/webidl/Node.webidl index c9fb7e77e2..bf2c1c4dee 100644 --- a/dom/webidl/Node.webidl +++ b/dom/webidl/Node.webidl @@ -38,7 +38,7 @@ interface Node : EventTarget { readonly attribute boolean isConnected; [Pure] readonly attribute Document? ownerDocument; - [Pure] + [Pure, Pref="dom.getRootNode.enabled"] Node getRootNode(optional GetRootNodeOptions options); [Pure] readonly attribute Node? parentNode; diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index ed3d9e1712..8cfac74a32 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4744,6 +4744,8 @@ pref("dom.push.enabled", false); pref("dom.push.loglevel", "error"); +pref("dom.getRootNode.enabled", false); + pref("dom.push.serverURL", "wss://push.services.mozilla.com/"); pref("dom.push.userAgentID", ""); From 9664369ac7e2c24894c5655b4ee2f714142f0a97 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 7 Aug 2020 14:21:59 -0400 Subject: [PATCH 15/16] [Pale-Moon] Issue MoonchildProductions/UXP#1628 - Remove extensions.update.background.url from preferences --- application/palemoon/app/profile/palemoon.js | 1 - 1 file changed, 1 deletion(-) diff --git a/application/palemoon/app/profile/palemoon.js b/application/palemoon/app/profile/palemoon.js index 3b048030b6..dac91154e4 100644 --- a/application/palemoon/app/profile/palemoon.js +++ b/application/palemoon/app/profile/palemoon.js @@ -195,7 +195,6 @@ pref("app.update.incompatible.mode", 0); // pref("extensions.update.enabled", true); pref("extensions.update.url", "https://@AM_DOMAIN@/?component=aus&@AM_AUS_ARGS@"); -pref("extensions.update.background.url", "https://@AM_DOMAIN@/?component=aus&@AM_AUS_ARGS@"); pref("extensions.update.interval", 86400); // Check for updates to Extensions and // Themes every day // Non-symmetric (not shared by extensions) extension-specific [update] preferences From 15c137e0b0b9d1935d273a55e22f263f38cb606f Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 7 Aug 2020 14:16:24 -0400 Subject: [PATCH 16/16] [Basilisk] Issue MoonchildProductions/UXP#1628 - Remove extensions.update.background.url from preferences --- application/basilisk/app/profile/basilisk.js | 1 - 1 file changed, 1 deletion(-) diff --git a/application/basilisk/app/profile/basilisk.js b/application/basilisk/app/profile/basilisk.js index ac55e21308..4a15fcded3 100644 --- a/application/basilisk/app/profile/basilisk.js +++ b/application/basilisk/app/profile/basilisk.js @@ -155,7 +155,6 @@ pref("app.update.idletime", 60); // pref("extensions.update.enabled", true); pref("extensions.update.url", "https://@AM_DOMAIN@/?component=aus&@AM_AUS_ARGS@"); -pref("extensions.update.background.url", "https://@AM_DOMAIN@/?component=aus&@AM_AUS_ARGS@"); pref("extensions.update.interval", 86400); // Check for updates to Extensions and // Themes every day // Non-symmetric (not shared by extensions) extension-specific [update] preferences