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 <object> and <input>, but I think
it's better to be consistent, since EnsureIntrinsicSizeAndRatio also doesn't
special-case <img> either.

Differential Revision: https://phabricator.services.mozilla.com/D11659

--HG--
extra : moz-landing-system : lando

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

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<imgIRequest> currentRequest = GetCurrentRequest()) {
+    uint32_t imageStatus;
+    return NS_SUCCEEDED(currentRequest->GetImageStatus(&imageStatus)) &&
+           (imageStatus & imgIRequest::STATUS_ERROR);
+  }
+
+  nsCOMPtr<nsIImageLoadingContent> 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<imgIRequest> currentRequest = GetCurrentRequest()) {
-    uint32_t imageStatus;
-    imageInvalid =
-      NS_SUCCEEDED(currentRequest->GetImageStatus(&imageStatus)) &&
-      (imageStatus & imgIRequest::STATUS_ERROR);
-  } else {
-    MOZ_ASSERT(mKind == Kind::ImageElement);
-
-    nsCOMPtr<nsIImageLoadingContent> 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 {

layout/generic/nsImageFrame.h

@@ -107,6 +107,7 @@ public:
 
   void ResponsiveContentDensityChanged();
   void SetupForContentURLRequest();
+  bool ShouldShowBrokenImageIcon() const;
 
 #ifdef ACCESSIBILITY
   virtual mozilla::a11y::AccType AccessibleType() override;

layout/reftests/image-element/broken-icon.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+  img {
+    display: inline-block;
+    width: 100px;
+    height: 100px;
+  }
+</style>
+<img src="broken">

layout/reftests/image-element/empty-src.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+  img {
+    display: inline-block;
+    width: 100px;
+    height: 100px;
+  }
+</style>
+<img src>

layout/reftests/image-element/invalid-src-2.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+  img {
+    display: inline-block;
+    width: 100px;
+    height: 100px;
+  }
+</style>
+<img src="http://yada yada">

layout/reftests/image-element/invalid-src.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+  img {
+    display: inline-block;
+    width: 100px;
+    height: 100px;
+  }
+</style>
+<img src="yada yada">

layout/reftests/image-element/no-src.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+  img {
+    display: inline-block;
+    width: 100px;
+    height: 100px;
+  }
+</style>
+<img>

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