On 09/13/2010 12:14 PM, Vincent Massol wrote:

On Sep 13, 2010, at 10:09 AM, Vincent Massol wrote:

...

On Sep 12, 2010, at 1:30 PM, mflorea (SVN) wrote:

...

Author: mflorea Date: 2010-09-12 13:30:15 +0200 (Sun, 12 Sep 2010) New Revision: 31053

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java Log: XWIKI-5479: Include image dimensions in the image URL for server side image resizing

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -50,7 +50,7 @@ *</ul> * Note that if the page title is empty or not defined then it defaults to %p. This is also the case * if the title cannot be retrieved for the document. - * + * * The default is "%p". Some examples: "%s.%p", "%w:%s.%p". * * @return the format to use to display link labels when the user hasn't specified a label @@ -63,4 +63,42 @@ * descriptor (ie defined by the macro author) will be used */ Properties getMacroCategories(); + + /** + * @return {@code true} to include the image dimensions extracted from the image parameters in the image URL, + * {@code false} otherwise; when image dimensions are included in the image URL the image can be resized + * on the server side before being downloaded. + * @since 2.5M2 + */ + boolean isIncludeImageDimensionsInImageURL();

I think is should be better named:

isImageDimensionsIncludedInImageURL()

I haven't followed too much but why do we need a flag for this? Why not always have the dimensions in the URL?

BTW how do I render an image that I want to resize in the browser but without resizing it on the server?

You can't from the XWiki syntax. If the image has its width/height specified then it is resized on the server: [[image:logo.jpg||width="150" height="100"]] Why would you want to download a 1024x768 image when you view it at only 150x100? If you call: getImageURL("Space.Page", "logo.jpg", {width:'150', height:'100'}) then you get a URL to a scaled version of logo.jpg image that has width 150 and height 100, which is IMO exactly what you are expecting.

For ex, if I want the image to be displayed in the page but I also want to be able to give a URL to the full size image?

Currently I don't see this as a real use case. What do you want to achieve with this? How is the user interacting with the image? As I said in a previous mail, resizing the image on the server is an optimization that needs to be transparent for the user. Do you think the user cares if the image is resized on the server or on the client if the result is almost the same? A developer that wants to display the original image in a light box or in a different page can still use the $doc.getAttachmentURL to get the URL to the original, full size, image. Thanks, Marius

Thanks -Vincent

...

Also, I agree with Thomas that this config should be internal to the xwiki implementation and not generic to all implementations.

Thanks -Vincent

...

+ + /** + * One way to improve page load speed is to resize images on the server side just before rendering the page. The + * rendering module can use the image width provided by the user to scale the image. When the user doesn't specify + * the image width the rendering module can limit the width of the image based on this configuration parameter. + *<p> + * The default value is {@code -1} which means image width is not limited by default. Use a value greater than 0 to + * limit the image width (pixels). Note that the aspect ratio is kept even when both the width and the height of the + * image are limited. + * + * @return the maximum image width when there's no user supplied width + * @see #isIncludeImageDimensionsInImageURL() + * @since 2.5M2 + */ + int getImageWidthLimit(); + + /** + * One way to improve page load speed is to resize images on the server side just before rendering the page. The + * rendering module can use the image height provided by the user to scale the image. When the user doesn't specify + * the image height the rendering module can limit the height of the image based on this configuration parameter. + *<p> + * The default value is {@code -1} which means image height is not limited by default. Use a value greater than 0 to + * limit the image height (pixels). Note that the aspect ratio is kept even when both the width and the height of + * the image are limited. + * + * @return the maximum image height when there's no user supplied height + * @see #isIncludeImageDimensionsInImageURL() + * @since 2.5M2 + */ + int getImageHeightLimit(); }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -45,6 +45,21 @@ private Properties macroCategories = new Properties();

/** + * @see #getImageWidthLimit() + */ + private int imageWidthLimit = -1; + + /** + * @see #getImageHeightLimit() + */ + private int imageHeightLimit = -1; + + /** + * @see #isIncludeImageDimensionsInImageURL() + */ + private boolean includeImageDimensionsInImageURL = true; + + /** * {@inheritDoc} * * @see org.xwiki.rendering.configuration.RenderingConfiguration#getLinkLabelFormat() @@ -84,4 +99,66 @@ // to work even without a configuration store. this.macroCategories.setProperty(macroId.toString(), category); } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageWidthLimit() + */ + public int getImageWidthLimit() + { + return this.imageWidthLimit; + } + + /** + * @param imageWidthLimit the maximum image width when there's no user supplied width + */ + public void setImageWidthLimit(int imageWidthLimit) + { + // This method is useful for those using the XWiki Rendering in standalone mode since it allows the rendering + // to work even without a configuration store. + this.imageWidthLimit = imageWidthLimit; + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageHeightLimit() + */ + public int getImageHeightLimit() + { + return this.imageHeightLimit; + } + + /** + * @param imageHeightLimit the maximum image height when there's no user supplied height + */ + public void setImageHeightLimit(int imageHeightLimit) + { + // This method is useful for those using the XWiki Rendering in standalone mode since it allows the rendering + // to work even without a configuration store. + this.imageHeightLimit = imageHeightLimit; + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#isIncludeImageDimensionsInImageURL() + */ + public boolean isIncludeImageDimensionsInImageURL() + { + return this.includeImageDimensionsInImageURL; + } + + /** + * @param includeImageDimensionsInImageURL {@code true} to include image dimensions extracted from the image + * parameters in the image URL so that the image can be resized on the server side before being + * downloaded, {@code false} otherwise + */ + public void setIncludeImageDimensionsInImageURL(boolean includeImageDimensionsInImageURL) + { + // This method is useful for those using the XWiki Rendering in standalone mode since it allows the rendering + // to work even without a configuration store. + this.includeImageDimensionsInImageURL = includeImageDimensionsInImageURL; + } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -42,10 +42,9 @@ * Default implementation for rendering images as XHTML. We handle both cases: *<ul> *<li>when inside a wiki (ie when an implementation of {@link WikiModel} is provided.</li> - *<li>when outside of a wiki. In this case we only handle external images and document images don't - * display anything.</li> + *<li>when outside of a wiki. In this case we only handle external images and document images don't display anything.</li> *</ul> - * + * * @version $Id$ * @since 2.0M3 */ @@ -68,6 +67,7 @@

/** * {@inheritDoc} + * * @see Initializable#initialize() */ public void initialize() throws InitializationException @@ -83,7 +83,7 @@

/** * {@inheritDoc} - * + * * @see XHTMLImageRenderer#setXHTMLWikiPrinter(XHTMLWikiPrinter) */ public void setXHTMLWikiPrinter(XHTMLWikiPrinter printer) @@ -103,7 +103,7 @@

/** * {@inheritDoc} - * + * * @see XHTMLImageRenderer#onImage(Image, boolean, Map) */ public void onImage(Image image, boolean isFreeStandingURI, Map<String, String> parameters) @@ -118,8 +118,9 @@ // that would not honor this contract. if (this.wikiModel != null) { DocumentImage documentImage = DocumentImage.class.cast(image); - imageURL = this.wikiModel.getAttachmentURL(documentImage.getDocumentName(), - documentImage.getAttachmentName()); + imageURL = + this.wikiModel.getImageURL(documentImage.getDocumentName(), documentImage.getAttachmentName(), + parameters); } else { throw new RuntimeException("Invalid Image type. In non wiki mode, all image types must be URL images."); }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,13 +19,15 @@ */ package org.xwiki.rendering.wiki;

+import java.util.Map; + import org.xwiki.component.annotation.ComponentRole;

/** * Bridge between the Rendering module and a Wiki Model. Contains wiki APIs required by Rendering classes such as - * Renderers. For example the XHTML Link Renderer needs to know if a wiki document exists in order to know how to - * generate the HTML (in order to display a question mark for non existing documents) and it also needs to get the - * URL pointing the wiki document. + * Renderers. For example the XHTML Link Renderer needs to know if a wiki document exists in order to know how to + * generate the HTML (in order to display a question mark for non existing documents) and it also needs to get the URL + * pointing the wiki document. * * @version $Id$ * @since 2.0M1 @@ -34,36 +36,48 @@ public interface WikiModel { /** - * @param documentName the document name of the document containing the attachment. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * @param documentName the document name of the document containing the attachment. The syntax used depends on the + * underlying wiki system used. For example for XWiki a valid documentName would be + * {@code wiki:Space.Page} * @param attachmentName the name of the attachment in the passed document wikip page * @return the URL to the attachment */ String getAttachmentURL(String documentName, String attachmentName);

/** - * @param documentName the document name as a String. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * Generate image specific URL. The difference with {@link #getAttachmentURL(String, String)} is that in some + * implementation we want to make a distinction between displayed image and a simple link targeting an attachment + * file. + * + * @param documentName the document name of the document containing the attachment. The syntax used depends on the + * underlying wiki system used. For example for XWiki a valid documentName would be + * {@code wiki:Space.Page} + * @param attachmentName the name of the attachment in the passed document wiki page + * @param parameters custom parameters + * @return the URL to the image + * @since 2.5M2 + */ + String getImageURL(String documentName, String attachmentName, Map<String, String> parameters); + + /** + * @param documentName the document name as a String. The syntax used depends on the underlying wiki system used. + * For example for XWiki a valid documentName would be {@code wiki:Space.Page} * @return true if the document exists and can be viewed or false otherwise */ boolean isDocumentAvailable(String documentName);

/** - * @param documentName the document name as a String. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * @param documentName the document name as a String. The syntax used depends on the underlying wiki system used. + * For example for XWiki a valid documentName would be {@code wiki:Space.Page} * @param anchor an anchor pointing to some place inside the document or null * @param queryString a query string specifying some parameters or null * @return the URL to view the specified wiki document */ String getDocumentViewURL(String documentName, String anchor, String queryString); - + /** - * @param documentName the document name as a String. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * @param documentName the document name as a String. The syntax used depends on the underlying wiki system used. + * For example for XWiki a valid documentName would be {@code wiki:Space.Page} * @param anchor an anchor pointing to some place inside the document or null * @param queryString a query string specifying some parameters or null * @return the URL to edit the specified wiki document

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml 2010-09-12 11:30:15 UTC (rev 31053) @@ -54,6 +54,12 @@ <artifactId>xwiki-core-script</artifactId> <version>${pom.version}</version> </dependency> +<dependency> +<!-- For parsing the CSS from the style attribute. --> +<groupId>net.sourceforge.cssparser</groupId> +<artifactId>cssparser</artifactId> +<version>0.9.5</version> +</dependency> <!-- Testing dependencies --> <dependency> <groupId>org.xwiki.platform</groupId>

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,13 +19,13 @@ */ package org.xwiki.rendering.internal.configuration;

+import java.util.Properties; + import org.xwiki.component.annotation.Component; import org.xwiki.component.annotation.Requirement; import org.xwiki.configuration.ConfigurationSource; import org.xwiki.rendering.configuration.RenderingConfiguration;

-import java.util.Properties; - /** * All configuration options for the rendering subsystem. * @@ -46,7 +46,7 @@ private static final String DEFAULT_LINK_LABEL_FORMAT = "%p";

/** - * Defines from where to read the rendering configuration data. + * Defines from where to read the rendering configuration data. */ @Requirement private ConfigurationSource configuration; @@ -58,16 +58,46 @@ */ public String getLinkLabelFormat() { - return this.configuration.getProperty(PREFIX + "linkLabelFormat", DEFAULT_LINK_LABEL_FORMAT); + return this.configuration.getProperty(PREFIX + "linkLabelFormat", DEFAULT_LINK_LABEL_FORMAT); }

/** * {@inheritDoc} - * + * * @see org.xwiki.rendering.configuration.RenderingConfiguration#getMacroCategories() */ public Properties getMacroCategories() { return this.configuration.getProperty(PREFIX + "macroCategories", Properties.class); } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageWidthLimit() + */ + public int getImageWidthLimit() + { + return this.configuration.getProperty(PREFIX + "imageWidthLimit", -1); + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageHeightLimit() + */ + public int getImageHeightLimit() + { + return this.configuration.getProperty(PREFIX + "imageHeightLimit", -1); + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#isIncludeImageDimensionsInImageURL() + */ + public boolean isIncludeImageDimensionsInImageURL() + { + return this.configuration.getProperty(PREFIX + "includeImageDimensionsInImageURL", true); + } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,17 +19,25 @@ */ package org.xwiki.rendering.internal.wiki;

+import java.io.IOException; +import java.io.StringReader; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; +import java.util.Map;

import org.apache.commons.lang.StringUtils; +import org.w3c.css.sac.InputSource; +import org.w3c.dom.css.CSSStyleDeclaration; import org.xwiki.bridge.DocumentAccessBridge; -import org.xwiki.model.reference.DocumentReference; import org.xwiki.component.annotation.Component; import org.xwiki.component.annotation.Requirement; +import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.rendering.configuration.RenderingConfiguration; import org.xwiki.rendering.wiki.WikiModel;

+import com.steadystate.css.parser.CSSOMParser; + /** * Implementation using the Document Access Bridge ({@link DocumentAccessBridge}). * @@ -39,14 +47,47 @@ @Component public class XWikiWikiModel implements WikiModel { + /** + * The suffix used to mark an amount of pixels. + */ + private static final String PIXELS = "px"; + + /** + * The name of the {@code width} image parameter. + */ + private static final String WIDTH = "width"; + + /** + * The name of the {@code height} image parameter. + */ + private static final String HEIGHT = "height"; + + /** + * The component used to access configuration parameters. + */ @Requirement + private RenderingConfiguration renderingConfiguration; + + /** + * The component used to access the underlying XWiki model. + */ + @Requirement private DocumentAccessBridge documentAccessBridge;

+ /** + * The component used to serialize entity references to strings. + */ @Requirement private EntityReferenceSerializer<String> entityReferenceSerializer;

/** + * The object used to parse the CSS from the image style parameter. + */ + private final CSSOMParser cssParser = new CSSOMParser(); + + /** * {@inheritDoc} + * * @see WikiModel#getAttachmentURL(String, String) */ public String getAttachmentURL(String documentName, String attachmentName) @@ -55,7 +96,112 @@ }

/** + * Extracts the specified image dimension from the image parameters. + * + * @param dimension either {@code width} or {@code height} + * @param imageParameters the image parameters; may include the {@code width}, {@code height} and {@code style} + * parameters + * @return the value of the passed dimension if it is specified in the image parameters, {@code null} otherwise + */ + private String getImageDimension(String dimension, Map<String, String> imageParameters) + { + // Check first if the style parameter contains information about the given dimension. In-line style has priority + // over the dimension parameters. + String value = null; + String style = imageParameters.get("style"); + if (StringUtils.isNotBlank(style)) { + try { + CSSStyleDeclaration sd = cssParser.parseStyleDeclaration(new InputSource(new StringReader(style))); + value = sd.getPropertyValue(dimension); + } catch (IOException e) { + // Ignore the style parameter. + } + } + if (StringUtils.isBlank(value)) { + // Fall back on the value of the dimension parameter. + value = imageParameters.get(dimension); + } + return value; + } + + /** + * Creates the query string that can be added to an image URL to resize the image on the server side. + * + * @param imageParameters image parameters, including width and height then they are specified + * @return the query string to be added to an image URL in order to resize the image on the server side + */ + private StringBuilder getImageURLQueryString(Map<String, String> imageParameters) + { + String width = StringUtils.chomp(getImageDimension(WIDTH, imageParameters), PIXELS); + String height = StringUtils.chomp(getImageDimension(HEIGHT, imageParameters), PIXELS); + boolean useHeight = StringUtils.isNotEmpty(height)&& StringUtils.isNumeric(height); + StringBuilder queryString = new StringBuilder(); + if (StringUtils.isEmpty(width) || !StringUtils.isNumeric(width)) { + // Width is unspecified or is not measured in pixels. + if (useHeight) { + // Height is specified in pixels. + queryString.append('&').append(HEIGHT).append('=').append(height); + } else { + // If image width and height are unspecified or if they are not expressed in pixels then limit the image + // size to best fit the rectangle specified in the configuration (keeping aspect ratio). + int widthLimit = renderingConfiguration.getImageWidthLimit(); + if (widthLimit> 0) { + queryString.append('&').append(WIDTH).append('=').append(widthLimit); + } + int heightLimit = renderingConfiguration.getImageHeightLimit(); + if (heightLimit> 0) { + queryString.append('&').append(HEIGHT).append('=').append(heightLimit); + } + if (widthLimit> 0&& heightLimit> 0) { + queryString.append("&keepAspectRatio=").append(true); + } + } + } else { + // Width is specified in pixels. + queryString.append('&').append(WIDTH).append('=').append(width); + if (useHeight) { + // Height is specified in pixels. + queryString.append('&').append(HEIGHT).append('=').append(height); + } + } + return queryString; + } + + /** * {@inheritDoc} + * + * @see org.xwiki.rendering.wiki.WikiModel#getImageURL(java.lang.String, java.lang.String, java.util.Map) + */ + public String getImageURL(String documentName, String attachmentName, Map<String, String> parameters) + { + String url = getAttachmentURL(documentName, attachmentName); + if (!renderingConfiguration.isIncludeImageDimensionsInImageURL()) { + return url; + } + + StringBuilder queryString = getImageURLQueryString(parameters); + if (queryString.length() == 0) { + return url; + } + + // Determine the insertion point. + int insertionPoint = url.lastIndexOf('#'); + if (insertionPoint< 0) { + // No fragment identifier. + insertionPoint = url.length(); + } + if (url.lastIndexOf('?', insertionPoint)< 0) { + // No query string. + queryString.setCharAt(0, '?'); + } + + // Insert the query string. + return new StringBuilder(url).insert(insertionPoint, queryString).toString(); + } + + /** + * {@inheritDoc} + * * @see WikiModel#isDocumentAvailable(String) */ public boolean isDocumentAvailable(String documentName) @@ -65,6 +211,7 @@

/** * {@inheritDoc} + * * @see WikiModel#getDocumentViewURL(String, String, String) */ public String getDocumentViewURL(String documentName, String anchor, String queryString) @@ -74,6 +221,7 @@

/** * {@inheritDoc} + * * @see WikiModel#getDocumentEditURL(String, String, String) */ public String getDocumentEditURL(String documentName, String anchor, String queryString) @@ -89,17 +237,17 @@ // See http://www.w3.org/TR/html40/appendix/notes.html#non-ascii-chars // TODO: Once the xwiki-url module is usable, refactor this code to use it and remove the need to // perform explicit encoding here. - modifiedQueryString = + modifiedQueryString = "parent=" + URLEncoder.encode(this.entityReferenceSerializer.serialize(reference), "UTF-8"); } catch (UnsupportedEncodingException e) { // Not supporting UTF-8 as a valid encoding for some reasons. We consider XWiki cannot work // without that encoding. - throw new RuntimeException("Failed to URL encode [" + this.entityReferenceSerializer.serialize( - reference) + "] using UTF-8.", e); + throw new RuntimeException("Failed to URL encode [" + + this.entityReferenceSerializer.serialize(reference) + "] using UTF-8.", e); } } } - + return this.documentAccessBridge.getURL(documentName, "create", modifiedQueryString, anchor); } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -22,9 +22,9 @@ import java.util.Properties;

import org.jmock.Expectations; -import org.xwiki.configuration.ConfigurationSource; import org.junit.Assert; import org.junit.Test; +import org.xwiki.configuration.ConfigurationSource; import org.xwiki.test.AbstractMockingComponentTestCase; import org.xwiki.test.annotation.MockingRequirement;

@@ -43,10 +43,13 @@ public void testGetLinkLabelFormat() throws Exception { final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); - getMockery().checking(new Expectations() {{ - allowing(source).getProperty("rendering.linkLabelFormat", "%p"); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.linkLabelFormat", "%p"); will(returnValue("%p")); - }}); + } + });

Assert.assertEquals("%p", this.configuration.getLinkLabelFormat()); } @@ -55,12 +58,60 @@ public void testGetMacroCategories() throws Exception { final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); - getMockery().checking(new Expectations() {{ - allowing(source).getProperty("rendering.macroCategories", Properties.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.macroCategories", Properties.class); will(returnValue(new Properties())); - }}); + } + });

Assert.assertNotNull(this.configuration.getMacroCategories()); Assert.assertEquals(0, this.configuration.getMacroCategories().size()); } + + @Test + public void testGetImageWidthLimit() throws Exception + { + final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.imageWidthLimit", -1); + will(returnValue(100)); + } + }); + + Assert.assertEquals(100, this.configuration.getImageWidthLimit()); + } + + @Test + public void testGetImageHeightLimit() throws Exception + { + final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.imageHeightLimit", -1); + will(returnValue(150)); + } + }); + + Assert.assertEquals(150, this.configuration.getImageHeightLimit()); + } + + @Test + public void testIsIncludeImageDimensionsInImageURL() throws Exception + { + final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.includeImageDimensionsInImageURL", true); + will(returnValue(false)); + } + }); + + Assert.assertFalse(this.configuration.isIncludeImageDimensionsInImageURL()); + } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,12 +19,20 @@ */ package org.xwiki.rendering.internal.wiki;

+import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import junit.framework.Assert; + import org.jmock.Expectations; -import org.jmock.Mockery; +import org.junit.Test; import org.xwiki.bridge.DocumentAccessBridge; import org.xwiki.model.reference.DocumentReference; -import org.xwiki.component.util.ReflectionUtils; import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.rendering.configuration.RenderingConfiguration; +import org.xwiki.test.AbstractMockingComponentTestCase; +import org.xwiki.test.annotation.MockingRequirement;

/** * Unit tests for {@link XWikiWikiModel}. @@ -32,31 +40,321 @@ * @version $Id$ * @since 2.0M1 */ -public class XWikiWikiModelTest +public class XWikiWikiModelTest extends AbstractMockingComponentTestCase { - @org.junit.Test + @MockingRequirement + private XWikiWikiModel wikiModel; + + @Test public void testGetDocumentEditURLWhenNoQueryStringSpecified() throws Exception { - Mockery mockery = new Mockery(); - XWikiWikiModel wikiModel = new XWikiWikiModel(); + final EntityReferenceSerializer< ?> entityReferenceSerializer = + getComponentManager().lookup(EntityReferenceSerializer.class); + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final DocumentReference documentReference = new DocumentReference("wiki", "Space", "Page"); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getCurrentDocumentReference(); + will(returnValue(documentReference)); + oneOf(entityReferenceSerializer).serialize(documentReference); + will(returnValue("wiki:Space.Page\u20AC"));

- final EntityReferenceSerializer mockEntityReferenceSerializer = mockery.mock(EntityReferenceSerializer.class); - ReflectionUtils.setFieldValue(wikiModel, "entityReferenceSerializer", mockEntityReferenceSerializer); + // The test is here: we verify that getURL is called with the query string already encoded since + // getURL() doesn't encode it. + oneOf(documentAccessBridge).getURL("Space.Page\u20AC", "create", "parent=wiki%3ASpace.Page%E2%82%AC", + "anchor"); + } + });

- final DocumentAccessBridge mockDocumentAccessBridge = mockery.mock(DocumentAccessBridge.class); - ReflectionUtils.setFieldValue(wikiModel, "documentAccessBridge", mockDocumentAccessBridge); + wikiModel.getDocumentEditURL("Space.Page\u20AC", "anchor", null); + }

- final DocumentReference docReference = new DocumentReference("wiki", "Space", "Page"); - mockery.checking(new Expectations() {{ - oneOf(mockDocumentAccessBridge).getCurrentDocumentReference(); will(returnValue(docReference)); - oneOf(mockEntityReferenceSerializer).serialize(docReference); will(returnValue("wiki:Space.Page\u20AC")); + /** + * Tests that the proper image URL is generated when both the width and the height image parameters are specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAttributesAreSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "100px"); + parameters.put("height", "50"); + Assert.assertEquals("?width=100&height=50", wikiModel.getImageURL("", "", parameters)); + }

- // The test is here: we verify that getURL is called with the query string already encoded since getURL() - // doesn't encode it. - oneOf(mockDocumentAccessBridge).getURL("Space.Page\u20AC", "create", "parent=wiki%3ASpace.Page%E2%82%AC", - "anchor"); - }}); + /** + * Tests that the proper image URL is generated when both the width and the height image parameters are specified + * but including them in the image URL is forbidden from the rendering configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenIncludingImageDimensionsIsForbidden() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(false)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "101px"); + parameters.put("height", "55px"); + Assert.assertEquals("", wikiModel.getImageURL("", "", parameters)); + }

- wikiModel.getDocumentEditURL("Space.Page\u20AC", "anchor", null); + /** + * Tests that the proper image URL is generated when both the width and the height CSS properties are specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightCSSPropertiesAreSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("style", "border: 1px; height: 30px; margin-top: 2em; width: 70px"); + Assert.assertEquals("?width=70&height=30", wikiModel.getImageURL("", "", parameters)); } + + /** + * Tests that the proper image URL is generated when only the width image parameter is specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenOnlyWidthAttributeIsSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "150"); + parameters.put("height", "30%"); + Assert.assertEquals("?width=150", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when only the height CSS property is specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenOnlyHeightCSSPropertyIsSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("style", "width: 5cm; height: 80px"); + Assert.assertEquals("?height=80", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the width and the height are unspecified and image size is + * limited in the configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAreUnspecifiedAndImageSizeIsLimited() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + oneOf(configuration).getImageWidthLimit(); + will(returnValue(200)); + oneOf(configuration).getImageHeightLimit(); + will(returnValue(170)); + } + }); + Map<String, String> parameters = Collections.emptyMap(); + Assert.assertEquals("?width=200&height=170&keepAspectRatio=true", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the width and the height are unspecified and only the + * image width is limited in the configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAreUnspecifiedAndOnlyImageWidthIsLimited() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + oneOf(configuration).getImageWidthLimit(); + will(returnValue(25)); + oneOf(configuration).getImageHeightLimit(); + will(returnValue(-1)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "45%"); + parameters.put("style", "height:10em"); + Assert.assertEquals("?width=25", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the width and the height are unspecified and the image + * size is not limited in the configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAreUnspecifiedAndImageSizeIsNotLimited() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + oneOf(configuration).getImageWidthLimit(); + will(returnValue(-1)); + oneOf(configuration).getImageHeightLimit(); + will(returnValue(-1)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("style", "bad CSS declaration"); + Assert.assertEquals("", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when the attachment URL has a fragment identifier. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenAttachmentURLHasFragmentIdentifier() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("test#fragment")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "23"); + Assert.assertEquals("test?width=23#fragment", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when the attachment URL has a query string and a fragment + * identifier. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenAttachmentURLHasQueryStringAndFragmentIdentifier() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("test?param=value#fragment")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("height", "17"); + Assert.assertEquals("test?param=value&height=17#fragment", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the style and the dimension parameters are specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothStyleAndDimensionParametersAreSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("height", "46"); + parameters.put("width", "101px"); + parameters.put("style", "width: 20%; height:75px"); + // Note that the style parameter take precedence over the dimension parameters and the width is actually 20% but + // we can't use it for resizing the image on the server side so it's omitted from the query string. + Assert.assertEquals("?height=75", wikiModel.getImageURL("", "", parameters)); + } }

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On 09/13/2010 02:34 PM, Vincent Massol wrote:

On Sep 13, 2010, at 2:17 PM, Marius Dumitru Florea wrote:

...

On 09/13/2010 12:14 PM, Vincent Massol wrote:

...

On Sep 13, 2010, at 10:09 AM, Vincent Massol wrote:

...

On Sep 12, 2010, at 1:30 PM, mflorea (SVN) wrote:

...

Author: mflorea Date: 2010-09-12 13:30:15 +0200 (Sun, 12 Sep 2010) New Revision: 31053

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java Log: XWIKI-5479: Include image dimensions in the image URL for server side image resizing

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -50,7 +50,7 @@ *</ul> * Note that if the page title is empty or not defined then it defaults to %p. This is also the case * if the title cannot be retrieved for the document. - * + * * The default is "%p". Some examples: "%s.%p", "%w:%s.%p". * * @return the format to use to display link labels when the user hasn't specified a label @@ -63,4 +63,42 @@ * descriptor (ie defined by the macro author) will be used */ Properties getMacroCategories(); + + /** + * @return {@code true} to include the image dimensions extracted from the image parameters in the image URL, + * {@code false} otherwise; when image dimensions are included in the image URL the image can be resized + * on the server side before being downloaded. + * @since 2.5M2 + */ + boolean isIncludeImageDimensionsInImageURL();

I think is should be better named:

isImageDimensionsIncludedInImageURL()

I haven't followed too much but why do we need a flag for this? Why not always have the dimensions in the URL?

...

BTW how do I render an image that I want to resize in the browser but without resizing it on the server?

You can't from the XWiki syntax. If the image has its width/height specified then it is resized on the server:

[[image:logo.jpg||width="150" height="100"]]

Why would you want to download a 1024x768 image when you view it at only 150x100? If you call:

getImageURL("Space.Page", "logo.jpg", {width:'150', height:'100'})

then you get a URL to a scaled version of logo.jpg image that has width 150 and height 100, which is IMO exactly what you are expecting.

...

For ex, if I want the image to be displayed in the page but I also want to be able to give a URL to the full size image?

Currently I don't see this as a real use case. What do you want to achieve with this? How is the user interacting with the image?

We do that all the time on xwiki.org. We display the image reduced but if the user clicks on it he sees the full size image.

Then just put a link around it.

...

As I said in a previous mail, resizing the image on the server is an optimization that needs to be transparent for the user. Do you think the user cares if the image is resized on the server or on the client if the result is almost the same?

A developer that wants to display the original image in a light box or in a different page can still use the $doc.getAttachmentURL to get the URL to the original, full size, image.

Thanks, Marius

-- Sergiu Dumitriu http://purl.org/net/sergiu/