bug 857089 - backout changeset b85353b6cc38 pending revised UI for the feature
authorJonathan Kew <jkew@mozilla.com>
Wed, 01 May 2013 10:12:41 +0100
changeset 141408 321a93ac614156a47e80a02d663ab5c9d79b7cf9
parent 141407 16ebb913027b63c52f3d03a18cdc73b3594af2e0
child 141409 da9dc0f5752f058a2400191c2c406d916714d846
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs857089
milestone23.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 857089 - backout changeset b85353b6cc38 pending revised UI for the feature
content/html/document/public/nsIImageDocument.idl
content/html/document/src/ImageDocument.cpp
content/html/document/test/test_bug369370.html
layout/style/ImageDocument.css
--- a/content/html/document/public/nsIImageDocument.idl
+++ b/content/html/document/public/nsIImageDocument.idl
@@ -6,55 +6,41 @@
 #include "nsISupports.idl"
 
 /**
  * @status UNDER_DEVELOPMENT
  */
 
 interface imgIRequest;
 
-[scriptable, uuid(0A70F5AF-2A78-4B52-9D96-11FF2A091E88)]
+[scriptable, uuid(7b80eebc-c98e-4461-8bdb-6e3b6e828890)]
 interface nsIImageDocument : nsISupports {
 
   /* Whether the pref for image resizing has been set. */
   readonly attribute boolean imageResizingEnabled;
 
   /* Whether the image is overflowing visible area. */
   readonly attribute boolean imageIsOverflowing;
 
   /* Whether the image has been resized to fit visible area. */
   readonly attribute boolean imageIsResized;
 
-  /* Whether the image has been scaled to device-pixel size. */
-  readonly attribute boolean imageIsScaledToDevicePixels;
-
   /* The image request being displayed in the content area */
   readonly attribute imgIRequest imageRequest;
 
   /* Resize the image to fit visible area. */
   void shrinkToFit();
 
-  /* Restore image original size (1:1 image pixels : CSS px). */
+  /* Restore image original size. */
   void restoreImage();
 
-  /* Set image to display at 1:1 image pixel : device-pixel size.
-   * If CSS px == device pixels, this is equivalent to restoreImage().
-   */
-  void scaleToDevicePixels();
-
   /* Restore the image, trying to keep a certain pixel in the same position.
    * The coordinate system is that of the shrunken image.
    */
   void restoreImageTo(in long x, in long y);
 
-  /* Scale to 1:1 device-pixel size, trying to keep a certain pixel
-   * in the same position.
-   * The coordinate system is that of the shrunken image.
-   */
-  void scaleToDevicePixelsTo(in long x, in long y);
-
   /* A helper method for switching between states.
    * The switching logic is as follows. If the image has been resized
    * restore image original size, otherwise if the image is overflowing
    * current visible area resize the image to fit the area.
    */
   void toggleImageSize();
 };  
--- a/content/html/document/src/ImageDocument.cpp
+++ b/content/html/document/src/ImageDocument.cpp
@@ -36,17 +36,16 @@
 #include "nsURILoader.h"
 #include "nsIDocShell.h"
 #include "nsIContentViewer.h"
 #include "nsIMarkupDocumentViewer.h"
 #include "nsThreadUtils.h"
 #include "nsIScrollableFrame.h"
 #include "nsContentUtils.h"
 #include "mozilla/dom/Element.h"
-#include "mozilla/dom/HTMLImageElement.h"
 #include "mozilla/Preferences.h"
 #include <algorithm>
 
 #define AUTOMATIC_IMAGE_RESIZING_PREF "browser.enable_automatic_image_resizing"
 #define CLICK_IMAGE_RESIZING_PREF "browser.enable_click_image_resizing"
 //XXX A hack needed for Firefox's site specific zoom.
 #define SITE_SPECIFIC_ZOOM "browser.zoom.siteSpecific"
 
@@ -106,47 +105,29 @@ public:
   virtual nsXPCClassInfo* GetClassInfo();
 protected:
   virtual nsresult CreateSyntheticDocument();
 
   nsresult CheckOverflowing(bool changeState);
 
   void UpdateTitleAndCharset();
 
-  enum eScaleOptions {
-    eDevicePixelScale,
-    eCSSPixelScale
-  };
-  nsresult ScrollImageTo(int32_t aX, int32_t aY, eScaleOptions aScaleOption);
+  nsresult ScrollImageTo(int32_t aX, int32_t aY, bool restoreImage);
 
-  float GetDevicePixelSizeInCSSPixels() {
-    nsIPresShell *shell = GetShell();
-    if (!shell) {
-      return 1.0f;
-    }
-    return shell->GetPresContext()->DevPixelsToFloatCSSPixels(1);
-  }
-
-  float GetShrinkToFitRatio() {
+  float GetRatio() {
     return std::min(mVisibleWidth / mImageWidth,
                     mVisibleHeight / mImageHeight);
   }
 
-  float GetCurrentRatio() {
-    return mImageIsScaledToDevicePixels ? GetDevicePixelSizeInCSSPixels() :
-           mImageIsResized ? GetShrinkToFitRatio() : 1.0f;
-  }
-
   void ResetZoomLevel();
   float GetZoomLevel();
 
   enum eModeClasses {
     eNone,
     eShrinkToFit,
-    eScaleToDevicePixels,
     eOverflowing
   };
   void SetModeClass(eModeClasses mode);
 
   nsresult OnStartContainer(imgIRequest* aRequest, imgIContainer* aImage);
   nsresult OnStopRequest(imgIRequest *aRequest, nsresult aStatus);
 
   nsCOMPtr<nsIContent>          mImageContent;
@@ -154,22 +135,18 @@ protected:
   float                         mVisibleWidth;
   float                         mVisibleHeight;
   int32_t                       mImageWidth;
   int32_t                       mImageHeight;
 
   bool                          mResizeImageByDefault;
   bool                          mClickResizingEnabled;
   bool                          mImageIsOverflowing;
-  // mImageIsResized is true if the image is resized to fit the window;
-  // mImageIsScaledToDevicePixels is true if the image is scaled to
-  // the 1:1 device-pixel size.
-  // These two flags cannot both be true at the same time.
+  // mImageIsResized is true if the image is currently resized
   bool                          mImageIsResized;
-  bool                          mImageIsScaledToDevicePixels;
   // mShouldResize is true if the image should be resized when it doesn't fit
   // mImageIsResized cannot be true when this is false, but mImageIsResized
   // can be false when this is true
   bool                          mShouldResize;
   bool                          mFirstResize;
   // mObservingImageLoader is true while the observer is set.
   bool                          mObservingImageLoader;
 
@@ -416,23 +393,16 @@ ImageDocument::GetImageIsOverflowing(boo
 NS_IMETHODIMP
 ImageDocument::GetImageIsResized(bool* aImageIsResized)
 {
   *aImageIsResized = mImageIsResized;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-ImageDocument::GetImageIsScaledToDevicePixels(bool* aImageIsScaledToDevPix)
-{
-  *aImageIsScaledToDevPix = mImageIsScaledToDevicePixels;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 ImageDocument::GetImageRequest(imgIRequest** aImageRequest)
 {
   nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mImageContent);
   if (imageLoader) {
     return imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
                                    aImageRequest);
   }
 
@@ -449,176 +419,98 @@ ImageDocument::ShrinkToFit()
   if (GetZoomLevel() != mOriginalZoomLevel && mImageIsResized &&
       !nsContentUtils::IsChildOfSameType(this)) {
     return NS_OK;
   }
 
   // Keep image content alive while changing the attributes.
   nsCOMPtr<nsIContent> imageContent = mImageContent;
   nsCOMPtr<nsIDOMHTMLImageElement> image = do_QueryInterface(mImageContent);
-  image->SetWidth(std::max(1, NSToCoordFloor(GetShrinkToFitRatio() * mImageWidth)));
-  image->SetHeight(std::max(1, NSToCoordFloor(GetShrinkToFitRatio() * mImageHeight)));
+  image->SetWidth(std::max(1, NSToCoordFloor(GetRatio() * mImageWidth)));
+  image->SetHeight(std::max(1, NSToCoordFloor(GetRatio() * mImageHeight)));
+  
+  // The view might have been scrolled when zooming in, scroll back to the
+  // origin now that we're showing a shrunk-to-window version.
+  (void) ScrollImageTo(0, 0, false);
 
   SetModeClass(eShrinkToFit);
-
+  
   mImageIsResized = true;
-
+  
   UpdateTitleAndCharset();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ImageDocument::RestoreImageTo(int32_t aX, int32_t aY)
 {
-  return ScrollImageTo(aX, aY, eCSSPixelScale);
-}
-
-NS_IMETHODIMP
-ImageDocument::ScaleToDevicePixelsTo(int32_t aX, int32_t aY)
-{
-  return ScrollImageTo(aX, aY, eDevicePixelScale);
+  return ScrollImageTo(aX, aY, true);
 }
 
 nsresult
-ImageDocument::ScrollImageTo(int32_t aX, int32_t aY,
-                             eScaleOptions aScaleOption)
+ImageDocument::ScrollImageTo(int32_t aX, int32_t aY, bool restoreImage)
 {
-  // Here, (aX, aY) is a position (in CSS pixels) within the displayed image
-  // (at its current scale) that we want to place as close as possible to the
-  // same position on screen after rescaling according to aScaleOption.
+  float ratio = GetRatio();
 
-  nsIPresShell *shell = GetShell();
-  if (!shell) {
-    return NS_OK;
-  }
-
-  nsIScrollableFrame* sf = shell->GetRootScrollFrameAsScrollable();
-  if (!sf) {
-    return NS_OK;
+  if (restoreImage) {
+    RestoreImage();
+    FlushPendingNotifications(Flush_Layout);
   }
 
-  // To figure out the unscaled image pixel location of interest, we need to
-  // undo the scaling from image pixels to the current display.
-  float ratio = GetCurrentRatio();
-  float imageX = aX / ratio;
-  float imageY = aY / ratio;
-
-  // And to preserve its position on screen, we'll need to account for the
-  // original position of the client rect, before it is moved/resized by
-  // rescaling the image.
-  nsRefPtr<nsClientRect> clientRect =
-    mImageContent->AsElement()->GetBoundingClientRect();
-  float origLeft = clientRect->Left();
-  float origTop = clientRect->Top();
-
-  // Update scaling and flush layout, so that we can get the new client rect.
-  switch (aScaleOption) {
-  case eDevicePixelScale:
-    ScaleToDevicePixels();
-    break;
-  case eCSSPixelScale:
-    RestoreImage();
-    break;
-  }
-  FlushPendingNotifications(Flush_Layout);
+  nsIPresShell *shell = GetShell();
+  if (!shell)
+    return NS_OK;
 
-  nsPoint scroll = sf->GetScrollPosition();
-
-  // Now update scroll to put the desired image pixel (at the new scale) at
-  // its original position, adjusting for the possibly-moved client rect.
-  clientRect = mImageContent->AsElement()->GetBoundingClientRect();
-  float clientLeft = clientRect->Left();
-  float clientTop = clientRect->Top();
-  ratio = GetCurrentRatio();
+  nsIScrollableFrame* sf = shell->GetRootScrollFrameAsScrollable();
+  if (!sf)
+    return NS_OK;
 
-  // Here (client{Left,Top} + image{X,Y}*ratio) is the new viewport-relative
-  // position of the click point in CSS px, while (orig{Left,Top} + a{X,Y}) is
-  // its old viewport-relative position. Adjust scroll pos by the difference.
-  scroll.x +=
-    nsPresContext::CSSPixelsToAppUnits((clientLeft + imageX * ratio) -
-                                       (origLeft + aX));
-  scroll.y +=
-    nsPresContext::CSSPixelsToAppUnits((clientTop + imageY * ratio) -
-                                       (origTop + aY));
-
-  // Finally set the scroll position (note that scrolling will be pinned to
-  // the limits of the frame's scrollable range, so we don't need to check
-  // for negative or excessively large scroll values here).
-  sf->ScrollTo(scroll, nsIScrollableFrame::INSTANT);
-
+  nsRect portRect = sf->GetScrollPortRect();
+  sf->ScrollTo(nsPoint(nsPresContext::CSSPixelsToAppUnits(aX/ratio) - portRect.width/2,
+                       nsPresContext::CSSPixelsToAppUnits(aY/ratio) - portRect.height/2),
+               nsIScrollableFrame::INSTANT);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ImageDocument::RestoreImage()
 {
   if (!mImageContent) {
     return NS_OK;
   }
   // Keep image content alive while changing the attributes.
   nsCOMPtr<nsIContent> imageContent = mImageContent;
   imageContent->UnsetAttr(kNameSpaceID_None, nsGkAtoms::width, true);
   imageContent->UnsetAttr(kNameSpaceID_None, nsGkAtoms::height, true);
-
-  // the "overflowing" mode really means "we could zoom out", so it applies
-  // if we are truly overflowing, OR if there's a device-pixel scale that
-  // would be smaller than the original (css-pixel) size
-  if (mImageIsOverflowing || GetDevicePixelSizeInCSSPixels() < 1.0f) {
+  
+  if (mImageIsOverflowing) {
     SetModeClass(eOverflowing);
   }
   else {
     SetModeClass(eNone);
   }
-
+  
   mImageIsResized = false;
-  mImageIsScaledToDevicePixels = false;
-
+  
   UpdateTitleAndCharset();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-ImageDocument::ScaleToDevicePixels()
-{
-  if (!mImageContent) {
-    return NS_OK;
-  }
-
-  float ratio = GetDevicePixelSizeInCSSPixels();
-  if (ratio == 1.0f) {
-    // if CSS px == device pix, we don't treat this as a separate scale option
-    return RestoreImage();
-  }
-
-  nsCOMPtr<nsIDOMHTMLImageElement> image = do_QueryInterface(mImageContent);
-  image->SetWidth(std::max(1, NSToCoordFloor(ratio * mImageWidth)));
-  image->SetHeight(std::max(1, NSToCoordFloor(ratio * mImageHeight)));
-
-  SetModeClass(eScaleToDevicePixels);
-
-  mImageIsResized = false;
-  mImageIsScaledToDevicePixels = true;
-
-  UpdateTitleAndCharset();
-
-  return NS_OK;
-}
-
-// TODO: make this cycle through the new ScaleToDevicePixels size as well
-NS_IMETHODIMP
 ImageDocument::ToggleImageSize()
 {
   mShouldResize = true;
   if (mImageIsResized) {
     mShouldResize = false;
     ResetZoomLevel();
     RestoreImage();
-  } else if (mImageIsOverflowing) {
+  }
+  else if (mImageIsOverflowing) {
     ResetZoomLevel();
     ShrinkToFit();
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -669,22 +561,16 @@ ImageDocument::SetModeClass(eModeClasses
   mozilla::ErrorResult rv;
 
   if (mode == eShrinkToFit) {
     classList->Add(NS_LITERAL_STRING("shrinkToFit"), rv);
   } else {
     classList->Remove(NS_LITERAL_STRING("shrinkToFit"), rv);
   }
 
-  if (mode == eScaleToDevicePixels) {
-    classList->Add(NS_LITERAL_STRING("scaleToDevicePixels"), rv);
-  } else {
-    classList->Remove(NS_LITERAL_STRING("scaleToDevicePixels"), rv);
-  }
-
   if (mode == eOverflowing) {
     classList->Add(NS_LITERAL_STRING("overflowing"), rv);
   } else {
     classList->Remove(NS_LITERAL_STRING("overflowing"), rv);
   }
 }
 
 nsresult
@@ -725,85 +611,39 @@ ImageDocument::OnStopRequest(imgIRequest
 
 NS_IMETHODIMP
 ImageDocument::HandleEvent(nsIDOMEvent* aEvent)
 {
   nsAutoString eventType;
   aEvent->GetType(eventType);
   if (eventType.EqualsLiteral("resize")) {
     CheckOverflowing(false);
-    return NS_OK;
   }
-
-  if (eventType.EqualsLiteral("click") && mClickResizingEnabled) {
+  else if (eventType.EqualsLiteral("click") && mClickResizingEnabled) {
     ResetZoomLevel();
     mShouldResize = true;
-
-    float devPixelRatio = GetDevicePixelSizeInCSSPixels();
-    float shrinkToFitRatio = GetShrinkToFitRatio();
-
-    // Figure out the target pixel to use if the image does not
-    // completely fit in the window after resizing.
-    int32_t x = 0, y = 0;
-    nsCOMPtr<nsIDOMMouseEvent> event(do_QueryInterface(aEvent));
-    if (event) {
-      event->GetClientX(&x);
-      event->GetClientY(&y);
-      // Adjust for any current scroll amount to get a location within
-      // the image as a whole (but still at its scaled size)
-      nsRefPtr<nsClientRect> clientRect =
-        mImageContent->AsElement()->GetBoundingClientRect();
-      x -= NSToIntRound(clientRect->Left());
-      y -= NSToIntRound(clientRect->Top());
-    }
-
-    if (mImageIsResized || mImageIsScaledToDevicePixels) {
-      // if CSS px == dev pix, there's only one thing to do here
-      if (devPixelRatio == 1.0f) {
-        RestoreImageTo(x, y);
-        mShouldResize = false;
-        return NS_OK;
+    if (mImageIsResized) {
+      int32_t x = 0, y = 0;
+      nsCOMPtr<nsIDOMMouseEvent> event(do_QueryInterface(aEvent));
+      if (event) {
+        event->GetClientX(&x);
+        event->GetClientY(&y);
+        int32_t left = 0, top = 0;
+        nsCOMPtr<nsIDOMHTMLElement> htmlElement =
+          do_QueryInterface(mImageContent);
+        htmlElement->GetOffsetLeft(&left);
+        htmlElement->GetOffsetTop(&top);
+        x -= left;
+        y -= top;
       }
-
-      if (mImageIsResized) {
-        // currently at shrink-to-fit size;
-        // if scaling to device pixels would make it bigger, do that;
-        // else scale to original 1:1 size
-        if (devPixelRatio > shrinkToFitRatio) {
-          ScaleToDevicePixelsTo(x, y);
-        } else {
-          RestoreImageTo(x, y);
-        }
-        mShouldResize = false;
-      } else {
-        if (shrinkToFitRatio > devPixelRatio && shrinkToFitRatio < 1.0f) {
-          ShrinkToFit();
-        } else {
-          RestoreImageTo(x, y);
-          mShouldResize = false;
-        }
-      }
-      return NS_OK;
+      mShouldResize = false;
+      RestoreImageTo(x, y);
     }
-
-    if (mImageIsOverflowing) {
-      // If the image is overflowing, we will shrink to the smaller
-      // of our possible sizes
-      if (devPixelRatio < shrinkToFitRatio) {
-        ScaleToDevicePixelsTo(x, y);
-        mShouldResize = false;
-      } else {
-        ShrinkToFit();
-      }
-      return NS_OK;
-    }
-
-    if (devPixelRatio != 1.0f) {
-      ScaleToDevicePixelsTo(x, y);
-      mShouldResize = false;
+    else if (mImageIsOverflowing) {
+      ShrinkToFit();
     }
   }
 
   return NS_OK;
 }
 
 nsresult
 ImageDocument::CreateSyntheticDocument()
@@ -927,19 +767,19 @@ ImageDocument::UpdateTitleAndCharset()
       }
       typeStr = Substring(iter, end);
     } else {
       typeStr = mimeType;
     }
   }
 
   nsXPIDLString status;
-  if (mImageIsResized || mImageIsScaledToDevicePixels) {
+  if (mImageIsResized) {
     nsAutoString ratioStr;
-    ratioStr.AppendInt(NSToCoordFloor(GetCurrentRatio() * 100));
+    ratioStr.AppendInt(NSToCoordFloor(GetRatio() * 100));
 
     const PRUnichar* formatString[1] = { ratioStr.get() };
     mStringBundle->FormatStringFromName(NS_LITERAL_STRING("ScaledImage").get(),
                                         formatString, 1,
                                         getter_Copies(status));
   }
 
   static const char* const formatNames[4] = 
--- a/content/html/document/test/test_bug369370.html
+++ b/content/html/document/test/test_bug369370.html
@@ -41,57 +41,46 @@ https://bugzilla.mozilla.org/show_bug.cg
         // Image just loaded and is scaled to window size.
         is(img.width,  400, "image width");
         is(img.height, 300, "image height");
         is(kidDoc.body.scrollLeft,  0, "Checking scrollLeft");
         is(kidDoc.body.scrollTop,   0, "Checking scrollTop");
 
         // ========== test 1 ==========
         // Click in the upper left to zoom in
-        var event = makeClickFor(25, 25);
+        var event = makeClickFor(25,25);
         img.dispatchEvent(event);
         ok(true, "----- click 1 -----");
 
         is(img.width,  800, "image width");
         is(img.height, 600, "image height");
-
-        // The image pixel at (25, 25) in the scaled image will be
-        // (50, 50) in the original image. To maintain its screen
-        // location, the image should have been scrolled up and left
-        // by (25, 25) px at its new size.
-        is(kidDoc.body.scrollLeft,  25, "Checking scrollLeft");
-        is(kidDoc.body.scrollTop,   25, "Checking scrollTop");
+        is(kidDoc.body.scrollLeft,  0, "Checking scrollLeft");
+        is(kidDoc.body.scrollTop,   0, "Checking scrollTop");
 
         // ========== test 2 ==========
         // Click there again to zoom out
-        event = makeClickFor(25, 25);
+        event = makeClickFor(25,25);
         img.dispatchEvent(event);
         ok(true, "----- click 2 -----");
 
         is(img.width,  400, "image width");
         is(img.height, 300, "image height");
         is(kidDoc.body.scrollLeft,  0, "Checking scrollLeft");
         is(kidDoc.body.scrollTop,   0, "Checking scrollTop");
 
         // ========== test 3 ==========
         // Click in the lower right to zoom in
         event = makeClickFor(350, 250);
         img.dispatchEvent(event);
         ok(true, "----- click 3 -----");
 
         is(img.width,  800, "image width");
         is(img.height, 600, "image height");
-
-        // The image pixel at (350, 250) in the scaled image will be
-        // (700, 500) in the original image. To maintain its screen
-        // location, the image should have been scrolled up and left
-        // by (350, 250) px at its new size.
-
-        is(kidDoc.body.scrollLeft,  350, "Checking scrollLeft");
-        is(kidDoc.body.scrollTop,   250, "Checking scrollTop");
+        is(kidDoc.body.scrollLeft,  400, "Checking scrollLeft");
+        is(kidDoc.body.scrollTop,   300, "Checking scrollTop");
 
         // ========== test 4 ==========
         // Click there again to zoom out
         event = makeClickFor(350, 250);
         img.dispatchEvent(event);
         ok(true, "----- click 4 -----");
 
         is(img.width,  400, "image width");
--- a/layout/style/ImageDocument.css
+++ b/layout/style/ImageDocument.css
@@ -8,18 +8,17 @@
  * including those in frames.
 */
 
 @media not print {
   .overflowing {
     cursor: -moz-zoom-out;
   }
 
-  .shrinkToFit,
-  .scaleToDevicePixels {
+  .shrinkToFit {
     cursor: -moz-zoom-in;
   }
 }
 
 @media print {
   /* We must declare the image as a block element. If we stay as
   an inline element, our parent LineBox will be inline too and
   ignore the available height during reflow.