Thomas Broyer has submitted this change and it was merged.

Change subject: Adds isStandalone to ImageResource so Image can used an UnclippedState.
......................................................................


Adds isStandalone to ImageResource so Image can used an UnclippedState.

isStandalone should be true for "data:" URLs and non-inlined images
(preventInlining=true or repeatStyle=Both). In other words, it should
actually rarely be false (IE6/7).

Fixes issue 7403

Change-Id: I832fd71e17e447e642de134e7a556befa1a7f0f1
---
M tools/api-checker/config/gwt25_26userApi.conf
M user/src/com/google/gwt/resources/client/ImageResource.java
M user/src/com/google/gwt/resources/client/impl/ImageResourcePrototype.java
M user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
M user/src/com/google/gwt/user/client/ui/AbstractImagePrototype.java
M user/src/com/google/gwt/user/client/ui/Image.java
M user/src/com/google/gwt/user/client/ui/ImageResourceRenderer.java
M user/test/com/google/gwt/cell/client/ImageResourceCellTest.java
M user/test/com/google/gwt/resources/client/ImageResourceTest.java
M user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
M user/test/com/google/gwt/user/client/ui/ImageTest.java
11 files changed, 173 insertions(+), 17 deletions(-)

Approvals:
  Leeroy Jenkins: Verified
  Thomas Broyer: Looks good to me, approved



diff --git a/tools/api-checker/config/gwt25_26userApi.conf b/tools/api-checker/config/gwt25_26userApi.conf
index c151c41..82d5a92 100644
--- a/tools/api-checker/config/gwt25_26userApi.conf
+++ b/tools/api-checker/config/gwt25_26userApi.conf
@@ -138,6 +138,7 @@
 :com.google.gwt.benchmarks.client.impl\
 :com.google.gwt.user.client.ui.impl\
 :com.google.gwt.i18n.client.impl\
+:com.google.gwt.resources.client.impl\

 ##############################################
 #Api  whitelist
diff --git a/user/src/com/google/gwt/resources/client/ImageResource.java b/user/src/com/google/gwt/resources/client/ImageResource.java
index 5f72fb8..f1885ed 100644
--- a/user/src/com/google/gwt/resources/client/ImageResource.java
+++ b/user/src/com/google/gwt/resources/client/ImageResource.java
@@ -147,4 +147,10 @@
    * Return <code>true</code> if the image contains multiple frames.
    */
   boolean isAnimated();
+
+  /**
+ * Returns <code>true</code> if the image is standalone, <code>false</code>
+   * if it's a region of a composite image.
+   */
+  boolean isStandalone();
 }
diff --git a/user/src/com/google/gwt/resources/client/impl/ImageResourcePrototype.java b/user/src/com/google/gwt/resources/client/impl/ImageResourcePrototype.java
index 7a422d4..ffabd21 100644
--- a/user/src/com/google/gwt/resources/client/impl/ImageResourcePrototype.java +++ b/user/src/com/google/gwt/resources/client/impl/ImageResourcePrototype.java
@@ -26,6 +26,7 @@

   private final boolean animated;
   private final boolean lossy;
+  private final boolean standalone;
   private final String name;
   private final SafeUri url;
   private final int left;
@@ -37,7 +38,7 @@
    * Only called by generated code.
    */
public ImageResourcePrototype(String name, SafeUri url, int left, int top, int width, int height,
-      boolean animated, boolean lossy) {
+      boolean animated, boolean lossy, boolean standalone) {
     this.name = name;
     this.left = left;
     this.top = top;
@@ -46,6 +47,7 @@
     this.url = url;
     this.animated = animated;
     this.lossy = lossy;
+    this.standalone = standalone;
   }

   /**
@@ -95,4 +97,9 @@
   public boolean isLossy() {
     return lossy;
   }
+
+  @Override
+  public boolean isStandalone() {
+    return standalone;
+  }
 }
diff --git a/user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java b/user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
index 4509439..9e25e69 100644
--- a/user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
+++ b/user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
@@ -71,6 +71,11 @@
       localizedByImageResource = Maps.create();
     }

+    @Override
+    public boolean isStandalone() {
+      return false;
+    }
+
public LocalizedImage addImage(TreeLogger logger, ResourceContext context,
         ImageResourceDeclaration image) throws UnableToCompleteException,
         CannotBundleImageException {
@@ -192,6 +197,7 @@
       if (isExternal) {
         return "External: " + image.get();
       }
+      // test mirrored in prepare(), make sure to keep them in sync
if (image.isPreventInlining() || image.getRepeatStyle() == RepeatStyle.Both) {
         return "Unbundled: " + image.get();
       }
@@ -253,6 +259,8 @@
     protected String normalContentsFieldName;
     protected String rtlContentsFieldName;

+    public abstract boolean isStandalone();
+
     public abstract ImageRect getImageRect(ImageResourceDeclaration image);

     /**
@@ -293,6 +301,11 @@
       this.image = image;
       this.localized = localized;
       this.rect = rect;
+    }
+
+    @Override
+    public boolean isStandalone() {
+      return true;
     }

     @Override
@@ -487,7 +500,8 @@
           + urlExpressions[1] + " : " + urlExpressions[0] + "),");
     }
sw.println(rect.getLeft() + ", " + rect.getTop() + ", " + rect.getWidth() + ", " - + rect.getHeight() + ", " + rect.isAnimated() + ", " + rect.isLossy()); + + rect.getHeight() + ", " + rect.isAnimated() + ", " + rect.isLossy() + ", "
+        + bundle.isStandalone());

     sw.outdent();
     sw.print(")");
@@ -543,7 +557,8 @@
       localizedImage = bundledImage.addImage(logger, context, image);
       rect = bundledImage.getImageRect(image);
       displayed = bundledImage;
-      if (image.isPreventInlining()) {
+      // mirrors the "unbundled" case in BundleKey.
+ if (image.isPreventInlining() || image.getRepeatStyle() == RepeatStyle.Both) {
         cannotBundle = true;
       }
     } catch (CannotBundleImageException e) {
diff --git a/user/src/com/google/gwt/user/client/ui/AbstractImagePrototype.java b/user/src/com/google/gwt/user/client/ui/AbstractImagePrototype.java
index b40d913..a122b3b 100644
--- a/user/src/com/google/gwt/user/client/ui/AbstractImagePrototype.java
+++ b/user/src/com/google/gwt/user/client/ui/AbstractImagePrototype.java
@@ -71,6 +71,8 @@
    *         ImageResource
    */
   public static AbstractImagePrototype create(ImageResource resource) {
+ // for backwards compatilibity, and to keep things simple, we treat standalone resources the + // same as composite ones (which means using a clear gif with the image as the CSS background) return new ClippedImagePrototype(resource.getSafeUri(), resource.getLeft(), resource.getTop(),
         resource.getWidth(), resource.getHeight());
   }
diff --git a/user/src/com/google/gwt/user/client/ui/Image.java b/user/src/com/google/gwt/user/client/ui/Image.java
index d616f41..8a59e27 100644
--- a/user/src/com/google/gwt/user/client/ui/Image.java
+++ b/user/src/com/google/gwt/user/client/ui/Image.java
@@ -202,10 +202,12 @@

     @Override
     public void setUrl(Image image, SafeUri url) {
-      image.changeState(new UnclippedState(image));
- // Need to make sure we change the state before an onload event can fire,
-      // or handlers will be fired while we are in the old state.
-      image.setUrl(url);
+      image.changeState(new UnclippedState(image, url));
+    }
+
+    @Override
+    public void setUrl(Image image, SafeUri url, int width, int height) {
+      image.changeState(new UnclippedState(image, url, width, height));
     }

     @Override
@@ -299,6 +301,8 @@

     public abstract void setUrl(Image image, SafeUri url);

+ public abstract void setUrl(Image image, SafeUri url, int width, int height);
+
public abstract void setUrlAndVisibleRect(Image image, SafeUri url, int left, int top,
         int width, int height);

@@ -380,6 +384,12 @@
       setUrl(image, url);
     }

+    UnclippedState(Image image, SafeUri url, int width, int height) {
+      this(image, url);
+      getImageElement(image).setWidth(width);
+      getImageElement(image).setHeight(height);
+    }
+
     @Override
     public int getHeight(Image image) {
       return getImageElement(image).getHeight();
@@ -414,6 +424,13 @@
     public void setUrl(Image image, SafeUri url) {
       image.clearUnhandledEvent();
       getImageElement(image).setSrc(url.asString());
+    }
+
+    @Override
+    public void setUrl(Image image, SafeUri url, int width, int height) {
+      setUrl(image, url);
+      getImageElement(image).setWidth(width);
+      getImageElement(image).setHeight(height);
     }

     @Override
@@ -499,8 +516,14 @@
    * @param resource the ImageResource to be displayed
    */
   public Image(ImageResource resource) {
- this(resource.getURL(), resource.getLeft(), resource.getTop(), resource.getWidth(),
-        resource.getHeight());
+    if (resource.isStandalone()) {
+ changeState(new UnclippedState(this, resource.getSafeUri(), resource.getWidth(),
+          resource.getHeight()));
+    } else {
+ changeState(new ClippedState(this, resource.getSafeUri(), resource.getLeft(),
+          resource.getTop(), resource.getWidth(), resource.getHeight()));
+    }
+    setStyleName("gwt-Image");
   }

   /**
@@ -849,8 +872,12 @@
    * @param resource the ImageResource to display
    */
   public void setResource(ImageResource resource) {
- setUrlAndVisibleRect(resource.getSafeUri(), resource.getLeft(), resource.getTop(),
-        resource.getWidth(), resource.getHeight());
+    if (resource.isStandalone()) {
+ state.setUrl(this, resource.getSafeUri(), resource.getWidth(), resource.getHeight());
+    } else {
+ state.setUrlAndVisibleRect(this, resource.getSafeUri(), resource.getLeft(),
+          resource.getTop(), resource.getWidth(), resource.getHeight());
+    }
   }

   /**
diff --git a/user/src/com/google/gwt/user/client/ui/ImageResourceRenderer.java b/user/src/com/google/gwt/user/client/ui/ImageResourceRenderer.java
index 962e04f..c78a5ec 100644
--- a/user/src/com/google/gwt/user/client/ui/ImageResourceRenderer.java
+++ b/user/src/com/google/gwt/user/client/ui/ImageResourceRenderer.java
@@ -15,16 +15,31 @@
  */
 package com.google.gwt.user.client.ui;

+import com.google.gwt.core.client.GWT;
 import com.google.gwt.resources.client.ImageResource;
+import com.google.gwt.safehtml.client.SafeHtmlTemplates;
 import com.google.gwt.safehtml.shared.SafeHtml;
+import com.google.gwt.safehtml.shared.SafeUri;
 import com.google.gwt.text.shared.AbstractSafeHtmlRenderer;

 /**
- * Given an {@link ImageResource}, renders a span element to show it.
+ * Given an {@link ImageResource}, renders an element to show it.
  */
public class ImageResourceRenderer extends AbstractSafeHtmlRenderer<ImageResource> {
+
+  interface Template extends SafeHtmlTemplates {
+ @SafeHtmlTemplates.Template("<img src='{0}' border='0' width='{1}' height='{2}'>")
+    SafeHtml image(SafeUri imageUri, int width, int height);
+  }
+
+  private static final Template TEMPLATE = GWT.create(Template.class);
+
   @Override
   public SafeHtml render(ImageResource image) {
-    return AbstractImagePrototype.create(image).getSafeHtml();
+    if (image.isStandalone()) {
+ return TEMPLATE.image(image.getSafeUri(), image.getWidth(), image.getHeight());
+    } else {
+      return AbstractImagePrototype.create(image).getSafeHtml();
+    }
   }
 }
diff --git a/user/test/com/google/gwt/cell/client/ImageResourceCellTest.java b/user/test/com/google/gwt/cell/client/ImageResourceCellTest.java
index 9f3ab8b..0142fce 100644
--- a/user/test/com/google/gwt/cell/client/ImageResourceCellTest.java
+++ b/user/test/com/google/gwt/cell/client/ImageResourceCellTest.java
@@ -18,7 +18,7 @@
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.resources.client.ClientBundle;
 import com.google.gwt.resources.client.ImageResource;
-import com.google.gwt.user.client.ui.AbstractImagePrototype;
+import com.google.gwt.user.client.ui.ImageResourceRenderer;

 /**
  * Tests for {@link ImageResourceCell}.
@@ -56,7 +56,7 @@

   @Override
   protected String getExpectedInnerHtml() {
- return AbstractImagePrototype.create(getImages().prettyPiccy()).getHTML(); + return new ImageResourceRenderer().render(getImages().prettyPiccy()).asString();
   }

   @Override
diff --git a/user/test/com/google/gwt/resources/client/ImageResourceTest.java b/user/test/com/google/gwt/resources/client/ImageResourceTest.java
index 55141d6..225634a 100644
--- a/user/test/com/google/gwt/resources/client/ImageResourceTest.java
+++ b/user/test/com/google/gwt/resources/client/ImageResourceTest.java
@@ -38,7 +38,15 @@

     @ImageOptions(preventInlining = true)
     @Source("32x32.png")
-    ImageResource i32x32();
+    ImageResource i32x32();
+
+    @ImageOptions(repeatStyle = RepeatStyle.Both)
+    @Source("16x16.png")
+    ImageResource i16x16RepeatBoth();
+
+    @ImageOptions(repeatStyle = RepeatStyle.Both)
+    @Source("32x32.png")
+    ImageResource i32x32RepeatBoth();
   }

   interface Resources extends ClientBundle {
@@ -163,6 +171,7 @@
     // Make sure that the large, lossy image isn't bundled with the rest
     assertTrue(((ImageResourcePrototype) lossy).isLossy());
     assertTrue(!i64.getSafeUri().equals(lossy.getSafeUri()));
+    assertTrue(lossy.isStandalone());

     assertEquals(16, r.i16x16Vertical().getWidth());
     assertEquals(16, r.i16x16Vertical().getHeight());
@@ -191,10 +200,32 @@
     // No image packing
     assertEquals(0, a.getTop());
     assertEquals(0, a.getLeft());
+    assertTrue(a.isStandalone());
     assertEquals(0, b.getTop());
     assertEquals(0, b.getLeft());
+    assertTrue(b.isStandalone());
   }
-
+
+  /**
+   * Tests that RepeatStyle.Both creates standalone images.
+   */
+  public void testRepeatStyleBoth() {
+    ExternalResources r = GWT.create(ExternalResources.class);
+    ImageResource a = r.i16x16RepeatBoth();
+    ImageResource b = r.i32x32RepeatBoth();
+
+    // Should be fetched from different URLs
+    assertFalse(a.getURL().equals(b.getURL()));
+
+    // No image packing
+    assertEquals(0, a.getTop());
+    assertEquals(0, a.getLeft());
+    assertTrue(a.isStandalone());
+    assertEquals(0, b.getTop());
+    assertEquals(0, b.getLeft());
+    assertTrue(b.isStandalone());
+  }
+
   @SuppressWarnings("deprecation")
   public void testSafeUri() {
     Resources r = GWT.create(Resources.class);
diff --git a/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java b/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
index 758bb69..b232e15 100644
--- a/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
+++ b/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
@@ -20,6 +20,8 @@
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.ParagraphElement;
 import com.google.gwt.dom.client.SpanElement;
+import com.google.gwt.junit.DoNotRunWith;
+import com.google.gwt.junit.Platform;
 import com.google.gwt.junit.client.GWTTestCase;
 import com.google.gwt.resources.client.ClientBundle;
 import com.google.gwt.resources.client.CssResource.NotStrict;
@@ -544,6 +546,12 @@
     assertEquals("funny characters \\ \" ' ' & < > > { }", t);
   }

+  /**
+ * Fails in all modes due to an HtmlUnit bug: offsetWidth always returns 1256.
+   * TODO(t.broyer): file a new HtmlUnit bug.
+   * Similar to http://sourceforge.net/p/htmlunit/bugs/1447/
+   */
+  @DoNotRunWith(Platform.HtmlUnitBug)
   public void testCustomImageClass() {
     ImageResource resource = widgetUi.prettyImage;
     Image widget = widgetUi.fooImage;
@@ -553,6 +561,12 @@
     assertEquals(resource.getLeft(), widget.getOriginLeft());
   }

+  /**
+ * Fails in all modes due to an HtmlUnit bug: offsetWidth always returns 1256.
+   * TODO(t.broyer): file a new HtmlUnit bug.
+   * Similar to http://sourceforge.net/p/htmlunit/bugs/1447/
+   */
+  @DoNotRunWith(Platform.HtmlUnitBug)
   public void testImageResourceInImageWidget() {
     ImageResource resource = widgetUi.prettyImage;
     Image widget = widgetUi.babyWidget;
diff --git a/user/test/com/google/gwt/user/client/ui/ImageTest.java b/user/test/com/google/gwt/user/client/ui/ImageTest.java
index 5fa9bf1..4845f30 100644
--- a/user/test/com/google/gwt/user/client/ui/ImageTest.java
+++ b/user/test/com/google/gwt/user/client/ui/ImageTest.java
@@ -26,8 +26,11 @@
 import com.google.gwt.junit.DoNotRunWith;
 import com.google.gwt.junit.Platform;
 import com.google.gwt.junit.client.GWTTestCase;
+import com.google.gwt.junit.client.WithProperties;
+import com.google.gwt.junit.client.WithProperties.Property;
 import com.google.gwt.resources.client.ClientBundle;
 import com.google.gwt.resources.client.ImageResource;
+import com.google.gwt.resources.client.ImageResource.ImageOptions;
 import com.google.gwt.user.client.Timer;

 /**
@@ -37,6 +40,10 @@
 public class ImageTest extends GWTTestCase {
   interface Bundle extends ClientBundle {
     ImageResource prettyPiccy();
+
+    @Source("prettyPiccy.png")
+    @ImageOptions(preventInlining = true)
+    ImageResource prettyPiccyStandalone();
   }

   private static class TestErrorHandler implements ErrorHandler {
@@ -609,17 +616,48 @@
     }.schedule(LOAD_EVENT_TIMEOUT);
    }

+  @WithProperties({
+    @Property(name = "ClientBundle.enableInlining", value = "false")
+  })
   public void testResourceConstructor() {
     Bundle b = GWT.create(Bundle.class);
     Image image = new Image(b.prettyPiccy());
     assertResourceWorked(image, b.prettyPiccy());
+
+    assertFalse(b.prettyPiccy().isStandalone());
+    assertEquals("clipped", getCurrentImageStateName(image));
   }

+  @WithProperties({
+    @Property(name = "ClientBundle.enableInlining", value = "false")
+  })
   public void testSetResource() {
     Bundle b = GWT.create(Bundle.class);
     Image image = new Image();
     image.setResource(b.prettyPiccy());
     assertResourceWorked(image, b.prettyPiccy());
+
+    assertFalse(b.prettyPiccy().isStandalone());
+    assertEquals("clipped", getCurrentImageStateName(image));
+  }
+
+  public void testStandaloneResourceConstructor() {
+    Bundle b = GWT.create(Bundle.class);
+    Image image = new Image(b.prettyPiccyStandalone());
+    assertResourceWorked(image, b.prettyPiccyStandalone());
+
+    assertTrue(b.prettyPiccyStandalone().isStandalone());
+    assertEquals("unclipped", getCurrentImageStateName(image));
+  }
+
+  public void testStandaloneSetResource() {
+    Bundle b = GWT.create(Bundle.class);
+    Image image = new Image();
+    image.setResource(b.prettyPiccyStandalone());
+    assertResourceWorked(image, b.prettyPiccyStandalone());
+
+    assertTrue(b.prettyPiccyStandalone().isStandalone());
+    assertEquals("unclipped", getCurrentImageStateName(image));
   }

   /**

--
To view, visit https://gwt-review.googlesource.com/2110
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I832fd71e17e447e642de134e7a556befa1a7f0f1
Gerrit-PatchSet: 8
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Thomas Broyer <t.bro...@gmail.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Leeroy Jenkins <jenk...@gwtproject.org>
Gerrit-Reviewer: Matthew Dempsky <mdemp...@google.com>
Gerrit-Reviewer: Matthew Dempsky <mdemp...@gwtproject.org>
Gerrit-Reviewer: Thomas Broyer <t.bro...@gmail.com>

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to