From 8e1f58b955c06aecaf2dd6c8f040903423ea4aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 13 Nov 2018 10:55:59 +0000 Subject: [PATCH] Bug 1506592 - Make sure to only display the broken image icon if there's a request at all. r=bzbarsky This is enough to fix the devtools regression and matches what other browsers do in the no-attribute case. Also, I think this change over all makes sense: it doesn't make any sense to display the broken image icon if there's no request, and we already assume in EnsureIntrinsicSizeAndRatio() that we don't paint the icon for those (and make the intrinsic size 0x0). We still show the border, which matches other UAs (note that devtools effectively masks the border away with mask-image). This technically also can change behavior of and , but I think it's better to be consistent, since EnsureIntrinsicSizeAndRatio also doesn't special-case either. Differential Revision: https://phabricator.services.mozilla.com/D11659 --HG-- extra : moz-landing-system : lando --- dom/base/nsIImageLoadingContent.idl | 2 +- layout/generic/nsImageFrame.cpp | 57 +++++++++---------- layout/generic/nsImageFrame.h | 1 + .../reftests/image-element/broken-icon.html | 9 +++ layout/reftests/image-element/empty-src.html | 9 +++ .../reftests/image-element/invalid-src-2.html | 9 +++ .../reftests/image-element/invalid-src.html | 9 +++ layout/reftests/image-element/no-src.html | 9 +++ layout/reftests/image-element/reftest.list | 5 ++ 9 files changed, 78 insertions(+), 32 deletions(-) create mode 100644 layout/reftests/image-element/broken-icon.html create mode 100644 layout/reftests/image-element/empty-src.html create mode 100644 layout/reftests/image-element/invalid-src-2.html create mode 100644 layout/reftests/image-element/invalid-src.html create mode 100644 layout/reftests/image-element/no-src.html diff --git a/dom/base/nsIImageLoadingContent.idl b/dom/base/nsIImageLoadingContent.idl index 6e6fb3cf6cdf..8b7d326eab46 100644 --- a/dom/base/nsIImageLoadingContent.idl +++ b/dom/base/nsIImageLoadingContent.idl @@ -71,7 +71,7 @@ interface nsIImageLoadingContent : imgINotificationObserver * the image was blocked. This status always refers to the * CURRENT_REQUEST load. */ - [noscript] readonly attribute short imageBlockingStatus; + [noscript, 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 85ed4083ba21..db3b8edef982 100644 --- a/layout/generic/nsImageFrame.cpp +++ b/layout/generic/nsImageFrame.cpp @@ -151,6 +151,29 @@ NS_NewImageFrameForGeneratedContentIndex(nsIPresShell* aPresShell, aStyle, nsImageFrame::Kind::ContentPropertyAtIndex); } +bool +nsImageFrame::ShouldShowBrokenImageIcon() const +{ + // NOTE(emilio, https://github.com/w3c/csswg-drafts/issues/2832): WebKit and + // Blink behave differently here for content: url(..), for now adapt to + // Blink's behavior. + if (mKind != Kind::ImageElement) { + return false; + } + + // check for broken images. valid null images (eg. img src="") are + // not considered broken because they have no image requests + if (nsCOMPtr currentRequest = GetCurrentRequest()) { + uint32_t imageStatus; + return NS_SUCCEEDED(currentRequest->GetImageStatus(&imageStatus)) && + (imageStatus & imgIRequest::STATUS_ERROR); + } + + nsCOMPtr loader = do_QueryInterface(mContent); + MOZ_ASSERT(loader); + return loader->GetImageBlockingStatus() != nsIContentPolicy::ACCEPT; +} + nsImageFrame* nsImageFrame::CreateContinuingFrame(nsIPresShell* aPresShell, ComputedStyle* aStyle) const @@ -936,37 +959,8 @@ nsImageFrame::EnsureIntrinsicSizeAndRatio() return; } - // NOTE(emilio, https://github.com/w3c/csswg-drafts/issues/2832): WebKit - // and Blink behave differently here for content: url(..), for now adapt to - // Blink's behavior. - const bool mayDisplayBrokenIcon = mKind == Kind::ImageElement; - if (!mayDisplayBrokenIcon) { - return; - } - // image request is null or image size not known, probably an - // invalid image specified - bool imageInvalid = false; - - // check for broken images. valid null images (eg. img src="") are - // not considered broken because they have no image requests - if (nsCOMPtr currentRequest = GetCurrentRequest()) { - uint32_t imageStatus; - imageInvalid = - NS_SUCCEEDED(currentRequest->GetImageStatus(&imageStatus)) && - (imageStatus & imgIRequest::STATUS_ERROR); - } else { - MOZ_ASSERT(mKind == Kind::ImageElement); - - nsCOMPtr loader = do_QueryInterface(mContent); - MOZ_ASSERT(loader); - // check if images are user-disabled (or blocked for other reasons) - int16_t imageBlockingStatus; - loader->GetImageBlockingStatus(&imageBlockingStatus); - imageInvalid = imageBlockingStatus != nsIContentPolicy::ACCEPT; - } - // invalid image specified. make the image big enough for the "broken" icon - if (imageInvalid) { + if (ShouldShowBrokenImageIcon()) { nscoord edgeLengthToUse = nsPresContext::CSSPixelsToAppUnits( ICON_SIZE + (2 * (ICON_PADDING + ALT_BORDER_WIDTH))); @@ -1494,7 +1488,8 @@ nsImageFrame::DisplayAltFeedback(gfxContext& aRenderingContext, ImgDrawResult result = ImgDrawResult::NOT_READY; // Check if we should display image placeholders - if (!gIconLoad->mPrefShowPlaceholders || + if (!ShouldShowBrokenImageIcon() || + !gIconLoad->mPrefShowPlaceholders || (isLoading && !gIconLoad->mPrefShowLoadingPlaceholder)) { result = ImgDrawResult::SUCCESS; } else { diff --git a/layout/generic/nsImageFrame.h b/layout/generic/nsImageFrame.h index a9887c21c74a..b65fbb6aef03 100644 --- a/layout/generic/nsImageFrame.h +++ b/layout/generic/nsImageFrame.h @@ -107,6 +107,7 @@ public: void ResponsiveContentDensityChanged(); void SetupForContentURLRequest(); + bool ShouldShowBrokenImageIcon() const; #ifdef ACCESSIBILITY virtual mozilla::a11y::AccType AccessibleType() override; diff --git a/layout/reftests/image-element/broken-icon.html b/layout/reftests/image-element/broken-icon.html new file mode 100644 index 000000000000..f2f6b7c451c6 --- /dev/null +++ b/layout/reftests/image-element/broken-icon.html @@ -0,0 +1,9 @@ + + + diff --git a/layout/reftests/image-element/empty-src.html b/layout/reftests/image-element/empty-src.html new file mode 100644 index 000000000000..36091cdc1909 --- /dev/null +++ b/layout/reftests/image-element/empty-src.html @@ -0,0 +1,9 @@ + + + diff --git a/layout/reftests/image-element/invalid-src-2.html b/layout/reftests/image-element/invalid-src-2.html new file mode 100644 index 000000000000..c385acca6dd0 --- /dev/null +++ b/layout/reftests/image-element/invalid-src-2.html @@ -0,0 +1,9 @@ + + + diff --git a/layout/reftests/image-element/invalid-src.html b/layout/reftests/image-element/invalid-src.html new file mode 100644 index 000000000000..26821d39b2ae --- /dev/null +++ b/layout/reftests/image-element/invalid-src.html @@ -0,0 +1,9 @@ + + + diff --git a/layout/reftests/image-element/no-src.html b/layout/reftests/image-element/no-src.html new file mode 100644 index 000000000000..6cc51078c020 --- /dev/null +++ b/layout/reftests/image-element/no-src.html @@ -0,0 +1,9 @@ + + + diff --git a/layout/reftests/image-element/reftest.list b/layout/reftests/image-element/reftest.list index 2d6923cb6e25..eff23b9b4290 100644 --- a/layout/reftests/image-element/reftest.list +++ b/layout/reftests/image-element/reftest.list @@ -45,5 +45,10 @@ HTTP == invalidate-1.html invalidate-1-ref.html == pattern-html-01.html pattern-html-01-ref.svg == pattern-html-02.html pattern-html-02-ref.svg == referenced-from-binding-01.html referenced-from-binding-01-ref.html +!= broken-icon.html no-src.html +!= broken-icon.html empty-src.html +== empty-src.html no-src.html +== broken-icon.html invalid-src.html +fails == invalid-src.html invalid-src-2.html # bug 1506804 fuzzy-if(skiaContent,0-1,0-30000) == mask-image-element.html mask-image-element-ref.html