From a7f14df010ddbb3ce83d835e346853452b802514 Mon Sep 17 00:00:00 2001 From: Cameron Kaiser Date: Mon, 18 May 2020 19:43:36 -0700 Subject: [PATCH] #601: empty img behaviour M1196668 M1616537 M1506592 (see also M1549742) --- dom/base/nsIImageLoadingContent.idl | 2 +- layout/generic/nsImageFrame.cpp | 125 +++++++++++++++++----------- layout/generic/nsImageFrame.h | 2 + 3 files changed, 78 insertions(+), 51 deletions(-) diff --git a/dom/base/nsIImageLoadingContent.idl b/dom/base/nsIImageLoadingContent.idl index 0a469c165..218e35b47 100644 --- a/dom/base/nsIImageLoadingContent.idl +++ b/dom/base/nsIImageLoadingContent.idl @@ -60,7 +60,7 @@ interface nsIImageLoadingContent : imgINotificationObserver * the image was blocked. This status always refers to the * CURRENT_REQUEST load. */ - readonly attribute short imageBlockingStatus; + [infallible] readonly attribute short imageBlockingStatus; /** * Used to register an image decoder observer. Typically, this will diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp index aa18e5548..8f39e807f 100644 --- a/layout/generic/nsImageFrame.cpp +++ b/layout/generic/nsImageFrame.cpp @@ -115,7 +115,7 @@ static bool HaveSpecifiedSize(const nsStylePosition* aStylePosition) // Decide whether we can optimize away reflows that result from the // image's intrinsic size changing. -inline bool HaveFixedSize(const nsHTMLReflowState& aReflowState) +static bool HaveFixedSize(const nsHTMLReflowState& aReflowState) { NS_ASSERTION(aReflowState.mStylePosition, "crappy reflowState - null stylePosition"); // Don't try to make this optimization when an image has percentages @@ -437,8 +437,23 @@ nsImageFrame::SourceRectToDest(const nsIntRect& aRect) (!(_state).HasAtLeastOneOfStates(NS_EVENT_STATE_BROKEN | NS_EVENT_STATE_USERDISABLED) && \ (_state).HasState(NS_EVENT_STATE_LOADING) && (_loadingOK))) -/* static */ -bool +static bool HasAltText(Element* aElement) +{ + // We always return some alternate text for , see + // nsCSSFrameConstructor::GetAlternateTextFor. + if (aElement->IsHTMLElement(nsGkAtoms::input)) { + return true; + } + + MOZ_ASSERT(aElement->IsHTMLElement(nsGkAtoms::img)); + nsAutoString altText; + return aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::alt, altText) && !altText.IsEmpty(); +} + +// Check if we want to use an image frame or just let the frame constructor make +// us into an inline. + +/* static */ bool nsImageFrame::ShouldCreateImageFrameFor(Element* aElement, nsStyleContext* aStyleContext) { @@ -449,44 +464,26 @@ nsImageFrame::ShouldCreateImageFrameFor(Element* aElement, return true; } - // Check if we want to use a placeholder box with an icon or just - // let the presShell make us into inline text. Decide as follows: - // - // - if our special "force icons" style is set, show an icon - // - else if our "do not show placeholders" pref is set, skip the icon - // - else: - // - if there is a src attribute, there is no alt attribute, - // and this is not an (which could not possibly have - // such an attribute), show an icon. - // - if QuirksMode, and the IMG has a size show an icon. - // - otherwise, skip the icon - bool useSizedBox; - + // If our special "force icons" style is set, show an icon if (aStyleContext->StyleUIReset()->mForceBrokenImageIcon) { - useSizedBox = true; - } - else if (gIconLoad && gIconLoad->mPrefForceInlineAltText) { - useSizedBox = false; - } - else if (aElement->HasAttr(kNameSpaceID_None, nsGkAtoms::src) && - !aElement->HasAttr(kNameSpaceID_None, nsGkAtoms::alt) && - !aElement->IsHTMLElement(nsGkAtoms::object) && - !aElement->IsHTMLElement(nsGkAtoms::input)) { - // Use a sized box if we have no alt text. This means no alt attribute - // and the node is not an object or an input (since those always have alt - // text). - useSizedBox = true; - } - else if (aStyleContext->PresContext()->CompatibilityMode() != - eCompatibility_NavQuirks) { - useSizedBox = false; - } - else { - // check whether we have specified size - useSizedBox = HaveSpecifiedSize(aStyleContext->StylePosition()); + return true; } - return useSizedBox; + // If our "do not show placeholders" pref is set, skip the icon + if (gIconLoad && gIconLoad->mPrefForceInlineAltText) { + return false; + } + + // If there is no Alt text, always create an image frame (regardless of src) + if (!HasAltText(aElement)) { + return true; + } + + if (aStyleContext->PresContext()->CompatibilityMode() == eCompatibility_NavQuirks) { + return HaveSpecifiedSize(aStyleContext->StylePosition()); + } + + return false; } nsresult @@ -767,6 +764,34 @@ nsImageFrame::PredictedDestRect(const nsRect& aFrameContentBox) StylePosition()); } +bool nsImageFrame::ShouldShowBrokenImageIcon() const +{ + bool imageBroken = false; + // Check for broken images. valid null images (eg. img src="") are + // not considered broken because they have no image requests + nsCOMPtr imageLoader = do_QueryInterface(mContent); + if (imageLoader) { + // is special, and it shouldn't draw the broken image icon, + // unlike the no-alt attribute or non-empty-alt-attribute case. + if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::alt)) { + nsAutoString altText; + mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::alt, altText); + if (altText.IsEmpty()) + return false; + } + + nsCOMPtr currentRequest; + imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST, + getter_AddRefs(currentRequest)); + uint32_t imageStatus; + imageBroken = + currentRequest && + NS_SUCCEEDED(currentRequest->GetImageStatus(&imageStatus)) && + (imageStatus & imgIRequest::STATUS_ERROR); + } + return imageBroken; +} + void nsImageFrame::EnsureIntrinsicSizeAndRatio() { @@ -776,22 +801,21 @@ nsImageFrame::EnsureIntrinsicSizeAndRatio() mIntrinsicSize.width.GetCoordValue() == 0 && mIntrinsicSize.height.GetUnit() == eStyleUnit_Coord && mIntrinsicSize.height.GetCoordValue() == 0) { - if (mImage) { UpdateIntrinsicSize(mImage); UpdateIntrinsicRatio(mImage); } else { - // image request is null or image size not known, probably an - // invalid image specified - // - make the image big enough for the icon (it may not be - // used if inline alt expansion is used instead) + // Image request is null or image size not known. if (!(GetStateBits() & NS_FRAME_GENERATED_CONTENT)) { - nscoord edgeLengthToUse = - nsPresContext::CSSPixelsToAppUnits( - ICON_SIZE + (2 * (ICON_PADDING + ALT_BORDER_WIDTH))); - mIntrinsicSize.width.SetCoordValue(edgeLengthToUse); - mIntrinsicSize.height.SetCoordValue(edgeLengthToUse); - mIntrinsicRatio.SizeTo(1, 1); + // Likely an invalid image. Check if we should display it as broken. + if (ShouldShowBrokenImageIcon()) { + nscoord edgeLengthToUse = + nsPresContext::CSSPixelsToAppUnits( + ICON_SIZE + (2 * (ICON_PADDING + ALT_BORDER_WIDTH))); + mIntrinsicSize.width.SetCoordValue(edgeLengthToUse); + mIntrinsicSize.height.SetCoordValue(edgeLengthToUse); + mIntrinsicRatio.SizeTo(1, 1); + } } } } @@ -1362,7 +1386,8 @@ nsImageFrame::DisplayAltFeedback(nsRenderingContext& aRenderingContext, DrawResult result = DrawResult::NOT_READY; // Check if we should display image placeholders - if (!gIconLoad->mPrefShowPlaceholders || + if (!ShouldShowBrokenImageIcon() || + !gIconLoad->mPrefShowPlaceholders || (isLoading && !gIconLoad->mPrefShowLoadingPlaceholder)) { result = DrawResult::SUCCESS; } else { diff --git a/layout/generic/nsImageFrame.h b/layout/generic/nsImageFrame.h index 4fc61d713..25814d27d 100644 --- a/layout/generic/nsImageFrame.h +++ b/layout/generic/nsImageFrame.h @@ -104,6 +104,8 @@ public: nsIAtom* aAttribute, int32_t aModType) override; + bool ShouldShowBrokenImageIcon() const; + #ifdef ACCESSIBILITY virtual mozilla::a11y::AccType AccessibleType() override; #endif