Bug 1149357: Adjust intrinsic size explicitly in both places. r?dholbert draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 16 May 2018 19:39:16 +0200
changeset 796215 3a91e814b3f0555557c2d93d949ab150c41ba021
parent 796087 1800b8895c08bc0c60302775dc0a4b5ea4deb310
child 796230 6e173ec34f57b42218b8ad8819ea67e80f6b9ff7
push id110182
push userbmo:emilio@crisal.io
push dateThu, 17 May 2018 08:51:23 +0000
reviewersdholbert
bugs1149357, 215083
milestone62.0a1
Bug 1149357: Adjust intrinsic size explicitly in both places. r?dholbert HTMLImageElement mangles it from GetNaturalSize when there's a srcset, but we weren't doing the proper thing on the normal mIntrinsicSize path. Move the adjustment to nsImageFrame for two reasons: * Prevents adding more dependencies from nsIImageLoadingContent, which otherwise would need to go away anyway in bug 215083. * Avoids having to duplicate the image orientation logic, since mImage is already an OrientedImage if needed. MozReview-Commit-ID: EA0n0TctZhN
dom/html/HTMLImageElement.h
layout/generic/nsImageFrame.cpp
layout/reftests/image/image-srcset-isize-ref.html
layout/reftests/image/image-srcset-isize.html
layout/reftests/image/reftest.list
--- a/dom/html/HTMLImageElement.h
+++ b/dom/html/HTMLImageElement.h
@@ -38,16 +38,21 @@ public:
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(HTMLImageElement,
                                            nsGenericHTMLElement)
 
   // nsISupports
   NS_DECL_ISUPPORTS_INHERITED
 
   virtual bool Draggable() const override;
 
+  ResponsiveImageSelector* GetResponsiveImageSelector()
+  {
+    return mResponsiveSelector.get();
+  }
+
   // Element
   virtual bool IsInteractiveHTMLContent(bool aIgnoreTabindex) const override;
 
   // EventTarget
   virtual void AsyncEventRunning(AsyncEventDispatcher* aEvent) override;
 
   NS_IMPL_FROMNODE_HTML_WITH_TAG(HTMLImageElement, img)
 
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -13,16 +13,18 @@
 #include "gfxUtils.h"
 #include "mozilla/ComputedStyle.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Encoding.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/Helpers.h"
 #include "mozilla/gfx/PathHelpers.h"
+#include "mozilla/dom/HTMLImageElement.h"
+#include "mozilla/dom/ResponsiveImageSelector.h"
 #include "mozilla/layers/WebRenderLayerManager.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/Unused.h"
 
 #include "nsCOMPtr.h"
 #include "nsFontMetrics.h"
 #include "nsIImageLoadingContent.h"
 #include "nsString.h"
@@ -289,29 +291,49 @@ nsImageFrame::Init(nsIContent*       aCo
     if (!HaveSpecifiedSize(StylePosition())) {
       categoryToBoostPriority |= imgIRequest::CATEGORY_SIZE_QUERY;
     }
 
     currentRequest->BoostPriority(categoryToBoostPriority);
   }
 }
 
+static void
+AdjustIntrinsicSize(nsIContent& aContent, nsSize& aSize)
+{
+  auto* image = HTMLImageElement::FromNode(aContent);
+  if (!image) {
+    return;
+  }
+
+  ResponsiveImageSelector* selector = image->GetResponsiveImageSelector();
+  if (!selector) {
+    return;
+  }
+
+  double density = selector->GetSelectedImageDensity();
+  MOZ_ASSERT(density > 0.0);
+  aSize.width = NSToIntRound(double(aSize.width) / density);
+  aSize.height = NSToIntRound(double(aSize.height) / density);
+}
+
 bool
 nsImageFrame::UpdateIntrinsicSize(imgIContainer* aImage)
 {
   MOZ_ASSERT(aImage, "null image");
   if (!aImage)
     return false;
 
   IntrinsicSize oldIntrinsicSize = mIntrinsicSize;
   mIntrinsicSize = IntrinsicSize();
 
   // Set intrinsic size to match aImage's reported intrinsic width & height.
   nsSize intrinsicSize;
   if (NS_SUCCEEDED(aImage->GetIntrinsicSize(&intrinsicSize))) {
+    AdjustIntrinsicSize(*mContent, intrinsicSize);
     // If the image has no intrinsic width, intrinsicSize.width will be -1, and
     // we can leave mIntrinsicSize.width at its default value of eStyleUnit_None.
     // Otherwise we use intrinsicSize.width. Height works the same way.
     if (intrinsicSize.width != -1)
       mIntrinsicSize.width.SetCoordValue(intrinsicSize.width);
     if (intrinsicSize.height != -1)
       mIntrinsicSize.height.SetCoordValue(intrinsicSize.height);
   } else {
@@ -873,33 +895,25 @@ nsImageFrame::ComputeSize(gfxContext *aR
   // below. See bug 1019840. We will fix this in bug 1141395.
 
   // Content may override our default dimensions. This is termed as overriding
   // the intrinsic size by the spec, but all other consumers of mIntrinsic*
   // values are being used to refer to the real/true size of the image data.
   if (imageLoader && imageLoader->CurrentRequestHasSize() && mImage &&
       intrinsicSize.width.GetUnit() == eStyleUnit_Coord &&
       intrinsicSize.height.GetUnit() == eStyleUnit_Coord) {
-    uint32_t width;
-    uint32_t height;
-    if (NS_SUCCEEDED(imageLoader->GetNaturalWidth(&width)) &&
-        NS_SUCCEEDED(imageLoader->GetNaturalHeight(&height))) {
-      nscoord appWidth = nsPresContext::CSSPixelsToAppUnits((int32_t)width);
-      nscoord appHeight = nsPresContext::CSSPixelsToAppUnits((int32_t)height);
-      // If this image is rotated, we'll need to transpose the natural
-      // width/height.
-      bool coordFlip;
-      if (StyleVisibility()->mImageOrientation.IsFromImage()) {
-        coordFlip = mImage->GetOrientation().SwapsWidthAndHeight();
-      } else {
-        coordFlip = StyleVisibility()->mImageOrientation.SwapsWidthAndHeight();
-      }
-      intrinsicSize.width.SetCoordValue(coordFlip ? appHeight : appWidth);
-      intrinsicSize.height.SetCoordValue(coordFlip ? appWidth : appHeight);
-    }
+    int32_t width = 0;
+    int32_t height = 0;
+    Unused << mImage->GetWidth(&width);
+    Unused << mImage->GetHeight(&height);
+    nsSize size(nsPresContext::CSSPixelsToAppUnits(width),
+                nsPresContext::CSSPixelsToAppUnits(height));
+    AdjustIntrinsicSize(*mContent, size);
+    intrinsicSize.width.SetCoordValue(size.width);
+    intrinsicSize.height.SetCoordValue(size.height);
   }
 
   return ComputeSizeWithIntrinsicDimensions(aRenderingContext, aWM,
                                             intrinsicSize, mIntrinsicRatio,
                                             aCBSize, aMargin, aBorder, aPadding,
                                             aFlags);
 }
 
new file mode 100644
--- /dev/null
+++ b/layout/reftests/image/image-srcset-isize-ref.html
@@ -0,0 +1,25 @@
+<!doctype html>
+<html reftest-zoom="2">
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<style>
+.image-container {
+  display: inline;
+}
+.content-container {
+  display: inline-block;
+}
+.flex-container {
+  align-items: center;
+  display: flex;
+}
+</style>
+<div class="flex-container">
+  <div class="image-container">
+    <img src="200.png" width="100">
+  </div>
+  <div class="content-container">
+    Should see me right by the side of the image.
+  </div>
+</div>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/image/image-srcset-isize.html
@@ -0,0 +1,41 @@
+<!doctype html>
+<html reftest-zoom="2" class="reftest-wait">
+<title>CSS Test: srcset intrinsic size isn't confused</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1149357">
+<style>
+.image-container {
+  display: inline;
+}
+.content-container {
+  display: inline-block;
+}
+.flex-container {
+  align-items: center;
+  display: flex;
+}
+</style>
+<script>
+  // reftest-zoom is only applied at onload, so ensure the source-selection
+  // has happened after that
+  function clearWait() {
+    document.documentElement.classList.remove("reftest-wait");
+  }
+  window.addEventListener("load", function() {
+    setTimeout(function() {
+      var img = document.querySelector("img");
+      img.onload = clearWait;
+      img.onerror = clearWait;
+      img.src = img.src;
+    }, 0);
+  });
+</script>
+<div class="flex-container">
+  <div class="image-container">
+    <img srcset="50.png 0.5x, 100.png 1x, 200.png 2x, 300.png 3x, 400.png">
+  </div>
+  <div class="content-container">
+    Should see me right by the side of the image.
+  </div>
+</div>
+</html>
--- a/layout/reftests/image/reftest.list
+++ b/layout/reftests/image/reftest.list
@@ -121,12 +121,14 @@ fuzzy(1,1) == image-orientation-backgrou
 == image-srcset-orientation-2x.html image-srcset-orientation-2x-ref.html
 == image-srcset-orientation-1x.html image-srcset-orientation-1x-ref.html
 == image-srcset-svg-3x.html image-srcset-svg-3x-ref.html
 == image-srcset-svg-2x.html image-srcset-svg-2x-ref.html
 == image-srcset-svg-1x.html image-srcset-svg-1x-ref.html
 == image-srcset-svg-default-2x.html image-srcset-svg-default-2x-ref.html
 == image-srcset-svg-default-1x.html image-srcset-svg-default-1x-ref.html
 
+== image-srcset-isize.html image-srcset-isize-ref.html
+
 == image-resize-percent-height.html image-resize-ref.html
 == image-resize-percent-width.html image-resize-ref.html
 
 == moz-broken-matching-1.html moz-broken-matching-1-ref.html