Merged #88 "Image diff could display before & after images"

13 files changed, 809 insertions, 28 deletions

diff --git a/src/main/java/com/gitblit/utils/DiffUtils.java b/src/main/java/com/gitblit/utils/DiffUtils.java
index f597b946..b30a203d 100644
--- a/src/main/java/com/gitblit/utils/DiffUtils.java
+++ b/src/main/java/com/gitblit/utils/DiffUtils.java
@@ -52,6 +52,27 @@ public class DiffUtils {
 	private static final Logger LOGGER = LoggerFactory.getLogger(DiffUtils.class);

 

 	/**

+	 * Callback interface for binary diffs. All the getDiff methods here take an optional handler;

+	 * if given and the {@link DiffOutputType} is {@link DiffOutputType#HTML HTML}, it is responsible

+	 * for displaying a binary diff.

+	 */

+	public interface BinaryDiffHandler {

+

+		/**

+		 * Renders a binary diff. The result must be valid HTML, it will be inserted into an HTML table cell.

+		 * May return {@code null} if the default behavior (which is typically just a textual note "Bnary

+		 * files differ") is desired.

+		 *

+		 * @param diffEntry

+		 *            current diff entry

+		 *

+		 * @return the rendered diff as HTML, or {@code null} if the default is desired.

+		 */

+		public String renderBinaryDiff(final DiffEntry diffEntry);

+

+	}

+

+	/**

 	 * Enumeration for the diff output types.

 	 */

 	public static enum DiffOutputType {

@@ -181,6 +202,23 @@ public class DiffUtils {
 	}

 

 	/**

+	 * Returns the complete diff of the specified commit compared to its primary parent.

+	 *

+	 * @param repository

+	 * @param commit

+	 * @param outputType

+	 * @param handler

+	 *            to use for rendering binary diffs if {@code outputType} is {@link DiffOutputType#HTML HTML}.

+	 *            May be {@code null}, resulting in the default behavior.

+	 * @return the diff

+	 */

+	public static DiffOutput getCommitDiff(Repository repository, RevCommit commit,

+			DiffOutputType outputType, BinaryDiffHandler handler) {

+		return getDiff(repository, null, commit, null, outputType, handler);

+	}

+

+

+	/**

 	 * Returns the diff for the specified file or folder from the specified

 	 * commit compared to its primary parent.

 	 *

@@ -196,6 +234,24 @@ public class DiffUtils {
 	}

 

 	/**

+	 * Returns the diff for the specified file or folder from the specified

+	 * commit compared to its primary parent.

+	 *

+	 * @param repository

+	 * @param commit

+	 * @param path

+	 * @param outputType

+	 * @param handler

+	 *            to use for rendering binary diffs if {@code outputType} is {@link DiffOutputType#HTML HTML}.

+	 *            May be {@code null}, resulting in the default behavior.

+	 * @return the diff

+	 */

+	public static DiffOutput getDiff(Repository repository, RevCommit commit, String path,

+			DiffOutputType outputType, BinaryDiffHandler handler) {

+		return getDiff(repository, null, commit, path, outputType, handler);

+	}

+

+	/**

 	 * Returns the complete diff between the two specified commits.

 	 *

 	 * @param repository

@@ -210,6 +266,23 @@ public class DiffUtils {
 	}

 

 	/**

+	 * Returns the complete diff between the two specified commits.

+	 *

+	 * @param repository

+	 * @param baseCommit

+	 * @param commit

+	 * @param outputType

+	 * @param handler

+	 *            to use for rendering binary diffs if {@code outputType} is {@link DiffOutputType#HTML HTML}.

+	 *            May be {@code null}, resulting in the default behavior.

+	 * @return the diff

+	 */

+	public static DiffOutput getDiff(Repository repository, RevCommit baseCommit, RevCommit commit,

+			DiffOutputType outputType, BinaryDiffHandler handler) {

+		return getDiff(repository, baseCommit, commit, null, outputType, handler);

+	}

+

+	/**

 	 * Returns the diff between two commits for the specified file.

 	 *

 	 * @param repository

@@ -225,6 +298,28 @@ public class DiffUtils {
 	 */

 	public static DiffOutput getDiff(Repository repository, RevCommit baseCommit, RevCommit commit,

 			String path, DiffOutputType outputType) {

+		return getDiff(repository, baseCommit, commit, path, outputType, null);

+	}

+

+	/**

+	 * Returns the diff between two commits for the specified file.

+	 *

+	 * @param repository

+	 * @param baseCommit

+	 *            if base commit is null the diff is to the primary parent of

+	 *            the commit.

+	 * @param commit

+	 * @param path

+	 *            if the path is specified, the diff is restricted to that file

+	 *            or folder. if unspecified, the diff is for the entire commit.

+	 * @param outputType

+	 * @param handler

+	 *            to use for rendering binary diffs if {@code outputType} is {@link DiffOutputType#HTML HTML}.

+	 *            May be {@code null}, resulting in the default behavior.

+	 * @return the diff

+	 */

+	public static DiffOutput getDiff(Repository repository, RevCommit baseCommit, RevCommit commit, String path, DiffOutputType outputType,

+			final BinaryDiffHandler handler) {

 		DiffStat stat = null;

 		String diff = null;

 		try {

@@ -233,7 +328,7 @@ public class DiffUtils {
 			DiffFormatter df;

 			switch (outputType) {

 			case HTML:

-				df = new GitBlitDiffFormatter(commit.getName(), path);

+				df = new GitBlitDiffFormatter(commit.getName(), path, handler);

 				break;

 			case PLAIN:

 			default:

diff --git a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java
index 5de9e50a..3c65267b 100644
--- a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java
+++ b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java
@@ -19,8 +19,10 @@ import static org.eclipse.jgit.lib.Constants.encode;
 import static org.eclipse.jgit.lib.Constants.encodeASCII;
 
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -34,6 +36,7 @@ import org.eclipse.jgit.diff.RawText;
 import org.eclipse.jgit.util.RawParseUtils;
 
 import com.gitblit.models.PathModel.PathChangeModel;
+import com.gitblit.utils.DiffUtils.BinaryDiffHandler;
 import com.gitblit.utils.DiffUtils.DiffStat;
 import com.gitblit.wicket.GitBlitWebApp;
 
@@ -71,7 +74,7 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 	 */
 	private static final int GLOBAL_DIFF_LIMIT = 20000;
 
-	private final ResettableByteArrayOutputStream os;
+	private final DiffOutputStream os;
 
 	private final DiffStat diffStat;
 
@@ -122,9 +125,45 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 	/** If {@link #truncated}, contains all entries skipped. */
 	private final List<DiffEntry> skipped = new ArrayList<DiffEntry>();
 
-	public GitBlitDiffFormatter(String commitId, String path) {
-		super(new ResettableByteArrayOutputStream());
-		this.os = (ResettableByteArrayOutputStream) getOutputStream();
+	/**
+	 * A {@link ResettableByteArrayOutputStream} that intercept the "Binary files differ" message produced
+	 * by the super implementation. Unfortunately the super implementation has far too many things private;
+	 * otherwise we'd just have re-implemented {@link GitBlitDiffFormatter#format(DiffEntry) format(DiffEntry)}
+	 * completely without ever calling the super implementation.
+	 */
+	private static class DiffOutputStream extends ResettableByteArrayOutputStream {
+
+		private static final String BINARY_DIFFERENCE = "Binary files differ\n";
+
+		private GitBlitDiffFormatter formatter;
+		private BinaryDiffHandler binaryDiffHandler;
+
+		public void setFormatter(GitBlitDiffFormatter formatter, BinaryDiffHandler handler) {
+			this.formatter = formatter;
+			this.binaryDiffHandler = handler;
+		}
+
+		@Override
+		public void write(byte[] b, int offset, int length) {
+			if (binaryDiffHandler != null
+					&& RawParseUtils.decode(Arrays.copyOfRange(b, offset, offset + length)).contains(BINARY_DIFFERENCE))
+			{
+				String binaryDiff = binaryDiffHandler.renderBinaryDiff(formatter.entry);
+				if (binaryDiff != null) {
+					byte[] bb = ("<tr><td colspan='4' align='center'>" + binaryDiff + "</td></tr>").getBytes(StandardCharsets.UTF_8);
+					super.write(bb, 0, bb.length);
+					return;
+				}
+			}
+			super.write(b, offset, length);
+		}
+
+	}
+
+	public GitBlitDiffFormatter(String commitId, String path, BinaryDiffHandler handler) {
+		super(new DiffOutputStream());
+		this.os = (DiffOutputStream) getOutputStream();
+		this.os.setFormatter(this, handler);
 		this.diffStat = new DiffStat(commitId);
 		// If we have a full commitdiff, install maxima to avoid generating a super-long diff listing that
 		// will only tax the browser too much.
@@ -217,7 +256,7 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 		super.format(ent);
 		if (!truncated) {
 			// Close the table
-			os.write("</tbody></table></div><br />\n".getBytes());
+			os.write("</tbody></table></div>\n".getBytes());
 		}
 	}
 
@@ -235,13 +274,7 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 	private void reset() {
 		if (!isOff) {
 			os.resetTo(startCurrent);
-			try {
-				os.write("<tr><td class='diff-cell' colspan='4'>".getBytes());
-				os.write(StringUtils.escapeForHtml(getMsg("gb.diffFileDiffTooLarge", "Diff too large"), false).getBytes());
-				os.write("</td></tr>\n".getBytes());
-			} catch (IOException ex) {
-				// Cannot happen with a ByteArrayOutputStream
-			}
+			writeFullWidthLine(getMsg("gb.diffFileDiffTooLarge", "Diff too large"));
 			totalNofLinesCurrent = totalNofLinesPrevious;
 			isOff = true;
 		}
@@ -277,13 +310,7 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 		default:
 			return;
 		}
-		try {
-			os.write("<tr><td class='diff-cell' colspan='4'>".getBytes());
-			os.write(StringUtils.escapeForHtml(message, false).getBytes());
-			os.write("</td></tr>\n".getBytes());
-		} catch (IOException ex) {
-			// Cannot happen with a ByteArrayOutputStream
-		}
+		writeFullWidthLine(message);
 	}
 
 	/**
@@ -355,6 +382,22 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 		}
 	}
 
+	/**
+	 * Writes a line spanning the full width of the code view, including the gutter.
+	 *
+	 * @param text
+	 *            to put on that line; will be HTML-escaped.
+	 */
+	private void writeFullWidthLine(String text) {
+		try {
+			os.write("<tr><td class='diff-cell' colspan='4'>".getBytes());
+			os.write(StringUtils.escapeForHtml(text, false).getBytes());
+			os.write("</td></tr>\n".getBytes());
+		} catch (IOException ex) {
+			// Cannot happen with a ByteArrayOutputStream
+		}
+	}
+
 	@Override
 	protected void writeLine(final char prefix, final RawText text, final int cur) throws IOException {
 		if (nofLinesCurrent++ == 0) {
@@ -453,6 +496,7 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 				if (gitLinkDiff) {
 					sb.append("</td></tr>");
 				}
+				sb.append('\n');
 			}
 		}
 		if (truncated) {
diff --git a/src/main/java/com/gitblit/utils/HtmlBuilder.java b/src/main/java/com/gitblit/utils/HtmlBuilder.java
new file mode 100644
index 00000000..6208ea82
--- /dev/null
+++ b/src/main/java/com/gitblit/utils/HtmlBuilder.java
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2014 Tom <tw201207@gmail.com>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.gitblit.utils;
+
+import org.jsoup.nodes.Document;
+import org.jsoup.nodes.Element;
+import org.jsoup.parser.Tag;
+
+/**
+ * Simple helper class to hide some common setup needed to use JSoup as an HTML generator.
+ * A {@link HtmlBuilder} has a root element that can be manipulated in all the usual JSoup
+ * ways (but not added to some {@link Document}); to generate HTML for that root element,
+ * the builder's {@link #toString()} method can be used. (Or one can invoke toString()
+ * directly on the root element.) By default, a HTML builder does not pretty-print the HTML.
+ *
+ * @author Tom <tw201207@gmail.com>
+ */
+public class HtmlBuilder {
+
+	private final Document shell;
+	private final Element root;
+
+	/**
+	 * Creates a new HTML builder with a root element with the given {@code tagName}.
+	 *
+	 * @param tagName
+	 *            of the {@link Element} to set as the root element.
+	 */
+	public HtmlBuilder(String tagName) {
+		this(new Element(Tag.valueOf(tagName), ""));
+	}
+
+	/**
+	 * Creates a new HTML builder for the given element.
+	 *
+	 * @param element
+	 *            to set as the root element of this HTML builder.
+	 */
+	public HtmlBuilder(Element element) {
+		shell = Document.createShell("");
+		shell.outputSettings().prettyPrint(false);
+		shell.body().appendChild(element);
+		root = element;
+	}
+
+	/** @return the root element of this HTML builder */
+	public Element getRoot() {
+		return root;
+	}
+
+	/** @return the root element of this HTML builder */
+	public Element root() {
+		return root;
+	}
+
+	/** @return whether this HTML builder will pretty-print the HTML generated by {@link #toString()} */
+	public boolean prettyPrint() {
+		return shell.outputSettings().prettyPrint();
+	}
+
+	/**
+	 * Sets whether this HTML builder will produce pretty-printed HTML in its {@link #toString()} method.
+	 *
+	 * @param pretty
+	 *            whether to pretty-print
+	 * @return the HTML builder itself
+	 */
+	public HtmlBuilder prettyPrint(boolean pretty) {
+		shell.outputSettings().prettyPrint(pretty);
+		return this;
+	}
+
+	/** @return the HTML for the root element. */
+	@Override
+	public String toString() {
+		return root.toString();
+	}
+
+	/** @return the {@link Document} used as generation shell. */
+	protected Document getShell() {
+		return shell;
+	}
+}
diff --git a/src/main/java/com/gitblit/wicket/pages/BasePage.html b/src/main/java/com/gitblit/wicket/pages/BasePage.html
index 7aa78360..b998428c 100644
--- a/src/main/java/com/gitblit/wicket/pages/BasePage.html
+++ b/src/main/java/com/gitblit/wicket/pages/BasePage.html
@@ -51,5 +51,6 @@
 		<!-- Include scripts at end for faster page loading -->

 		<script type="text/javascript" src="bootstrap/js/jquery.js"></script>

 		<script type="text/javascript" src="bootstrap/js/bootstrap.js"></script>		

+		<wicket:container wicket:id="bottomScripts"></wicket:container>

 	</body>

 </html>
\ No newline at end of file
diff --git a/src/main/java/com/gitblit/wicket/pages/BasePage.java b/src/main/java/com/gitblit/wicket/pages/BasePage.java
index b6967003..fbe5861b 100644
--- a/src/main/java/com/gitblit/wicket/pages/BasePage.java
+++ b/src/main/java/com/gitblit/wicket/pages/BasePage.java
@@ -35,6 +35,7 @@ import javax.servlet.http.HttpServletRequest;
 

 import org.apache.commons.io.IOUtils;

 import org.apache.wicket.Application;

+import org.apache.wicket.Component;

 import org.apache.wicket.Page;

 import org.apache.wicket.PageParameters;

 import org.apache.wicket.RedirectToUrlException;

@@ -42,6 +43,8 @@ import org.apache.wicket.markup.html.CSSPackageResource;
 import org.apache.wicket.markup.html.basic.Label;

 import org.apache.wicket.markup.html.link.ExternalLink;

 import org.apache.wicket.markup.html.panel.FeedbackPanel;

+import org.apache.wicket.markup.html.resources.JavascriptResourceReference;

+import org.apache.wicket.markup.repeater.RepeatingView;

 import org.apache.wicket.protocol.http.RequestUtils;

 import org.apache.wicket.protocol.http.WebResponse;

 import org.apache.wicket.protocol.http.servlet.ServletWebRequest;

@@ -75,11 +78,13 @@ public abstract class BasePage extends SessionPage {
 

 	public BasePage() {

 		super();

+		add(new RepeatingView("bottomScripts").setRenderBodyOnly(true));

 		customizeHeader();

 	}

 

 	public BasePage(PageParameters params) {

 		super(params);

+		add(new RepeatingView("bottomScripts").setRenderBodyOnly(true));

 		customizeHeader();

 	}

 

@@ -506,4 +511,38 @@ public abstract class BasePage extends SessionPage {
 		return sb.toString();

 	}

 

+	/**

+	 * Adds a HTML script element loading the javascript designated by the given path.

+	 *

+	 * @param scriptPath

+	 *            page-relative path to the Javascript resource; normally starts with "scripts/"

+	 */

+	protected void addBottomScript(String scriptPath) {

+		Component bottomScriptContainer = get("bottomScripts");

+		if (bottomScriptContainer instanceof RepeatingView) {

+			// Always true.

+			RepeatingView bottomScripts = (RepeatingView) bottomScriptContainer;

+			Label script = new Label(bottomScripts.newChildId(), "<script type='text/javascript' src='"

+					+ urlFor(new JavascriptResourceReference(this.getClass(), scriptPath)) + "'></script>\n");

+			bottomScripts.add(script.setEscapeModelStrings(false).setRenderBodyOnly(true));

+		}

+	}

+

+	/**

+	 * Adds a HTML script element containing the given code.

+	 *

+	 * @param code

+	 *            inline script code

+	 */

+	protected void addBottomScriptInline(String code) {

+		Component bottomScriptContainer = get("bottomScripts");

+		if (bottomScriptContainer instanceof RepeatingView) {

+			// Always true.

+			RepeatingView bottomScripts = (RepeatingView) bottomScriptContainer;

+			Label script = new Label(bottomScripts.newChildId(),

+					"<script type='text/javascript'>/*<![CDATA[*/\n" + code + "\n//]]>\n</script>\n");

+			bottomScripts.add(script.setEscapeModelStrings(false).setRenderBodyOnly(true));

+		}

+	}

+

 }

diff --git a/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java b/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java
index 9cc3eae1..71516ec8 100644
--- a/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java
@@ -15,12 +15,15 @@
  */

 package com.gitblit.wicket.pages;

 

+import java.util.List;

+

 import org.apache.wicket.PageParameters;

 import org.apache.wicket.markup.html.basic.Label;

 import org.apache.wicket.markup.html.link.BookmarkablePageLink;

 import org.eclipse.jgit.lib.Repository;

 import org.eclipse.jgit.revwalk.RevCommit;

 

+import com.gitblit.Keys;

 import com.gitblit.utils.DiffUtils;

 import com.gitblit.utils.DiffUtils.DiffOutputType;

 import com.gitblit.utils.JGitUtils;

@@ -43,16 +46,29 @@ public class BlobDiffPage extends RepositoryPage {
 		Repository r = getRepository();

 		RevCommit commit = getCommit();

 

+		final List<String> imageExtensions = app().settings().getStrings(Keys.web.imageExtensions);

+

 		String diff;

 		if (StringUtils.isEmpty(baseObjectId)) {

 			// use first parent

-			diff = DiffUtils.getDiff(r, commit, blobPath, DiffOutputType.HTML).content;

+			RevCommit parent = commit.getParentCount() == 0 ? null : commit.getParent(0);

+			ImageDiffHandler handler = new ImageDiffHandler(getContextUrl(), repositoryName,

+					parent.getName(), commit.getName(), imageExtensions);

+			diff = DiffUtils.getDiff(r, commit, blobPath, DiffOutputType.HTML, handler).content;

+			if (handler.getImgDiffCount() > 0) {

+				addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs

+			}

 			add(new BookmarkablePageLink<Void>("patchLink", PatchPage.class,

 					WicketUtils.newPathParameter(repositoryName, objectId, blobPath)));

 		} else {

 			// base commit specified

 			RevCommit baseCommit = JGitUtils.getCommit(r, baseObjectId);

-			diff = DiffUtils.getDiff(r, baseCommit, commit, blobPath, DiffOutputType.HTML).content;

+			ImageDiffHandler handler = new ImageDiffHandler(getContextUrl(), repositoryName,

+					baseCommit.getName(), commit.getName(), imageExtensions);

+			diff = DiffUtils.getDiff(r, baseCommit, commit, blobPath, DiffOutputType.HTML, handler).content;

+			if (handler.getImgDiffCount() > 0) {

+				addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs

+			}

 			add(new BookmarkablePageLink<Void>("patchLink", PatchPage.class,

 					WicketUtils.newBlobDiffParameter(repositoryName, baseObjectId, objectId,

 							blobPath)));

diff --git a/src/main/java/com/gitblit/wicket/pages/BlobPage.html b/src/main/java/com/gitblit/wicket/pages/BlobPage.html
index 3f9a959a..289c1498 100644
--- a/src/main/java/com/gitblit/wicket/pages/BlobPage.html
+++ b/src/main/java/com/gitblit/wicket/pages/BlobPage.html
@@ -40,9 +40,8 @@
   </wicket:link>

 </wicket:head>

 

+<body>

 <wicket:extend>

-<!-- need to specify body.onload -->

-<body onload="prettyPrint()">

 

 		<!-- blob nav links -->	

 		<div class="page_nav2">

@@ -61,6 +60,6 @@
 		<!--  blob image -->

 		<img wicket:id="blobImage" style="padding-top:5px;"></img>

 	

-</body>

 </wicket:extend>

+</body>

 </html>
\ No newline at end of file
diff --git a/src/main/java/com/gitblit/wicket/pages/BlobPage.java b/src/main/java/com/gitblit/wicket/pages/BlobPage.java
index 3c244f9a..df30e4cc 100644
--- a/src/main/java/com/gitblit/wicket/pages/BlobPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/BlobPage.java
@@ -137,6 +137,7 @@ public class BlobPage extends RepositoryPage {
 						table = missingBlob(blobPath, commit);

 					} else {

 						table = generateSourceView(source, extension, type == 1);

+						addBottomScriptInline("jQuery(prettyPrint);");

 					}

 					add(new Label("blobText", table).setEscapeModelStrings(false));

 					add(new Image("blobImage").setVisible(false));

@@ -150,6 +151,7 @@ public class BlobPage extends RepositoryPage {
 					table = missingBlob(blobPath, commit);

 				} else {

 					table = generateSourceView(source, null, false);

+					addBottomScriptInline("jQuery(prettyPrint);");

 				}

 				add(new Label("blobText", table).setEscapeModelStrings(false));

 				add(new Image("blobImage").setVisible(false));

diff --git a/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java b/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java
index 34ff5a29..e40af515 100644
--- a/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java
@@ -31,6 +31,7 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 
 import com.gitblit.Constants;
+import com.gitblit.Keys;
 import com.gitblit.models.GitNote;
 import com.gitblit.models.PathModel.PathChangeModel;
 import com.gitblit.models.SubmoduleModel;
@@ -59,8 +60,6 @@ public class CommitDiffPage extends RepositoryPage {
 
 		RevCommit commit = getCommit();
 
-		final DiffOutput diff = DiffUtils.getCommitDiff(r, commit, DiffOutputType.HTML);
-
 		List<String> parents = new ArrayList<String>();
 		if (commit.getParentCount() > 0) {
 			for (RevCommit parent : commit.getParents()) {
@@ -82,6 +81,14 @@ public class CommitDiffPage extends RepositoryPage {
 
 		add(new CommitHeaderPanel("commitHeader", repositoryName, commit));
 
+		final List<String> imageExtensions = app().settings().getStrings(Keys.web.imageExtensions);
+		final ImageDiffHandler handler = new ImageDiffHandler(getContextUrl(), repositoryName,
+				parents.isEmpty() ? null : parents.get(0), commit.getName(), imageExtensions);
+		final DiffOutput diff = DiffUtils.getCommitDiff(r, commit, DiffOutputType.HTML, handler);
+		if (handler.getImgDiffCount() > 0) {
+			addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs
+		}
+
 		// add commit diffstat
 		int insertions = 0;
 		int deletions = 0;
diff --git a/src/main/java/com/gitblit/wicket/pages/ComparePage.java b/src/main/java/com/gitblit/wicket/pages/ComparePage.java
index 3b8bb03e..c0141eba 100644
--- a/src/main/java/com/gitblit/wicket/pages/ComparePage.java
+++ b/src/main/java/com/gitblit/wicket/pages/ComparePage.java
@@ -37,6 +37,7 @@ import org.eclipse.jgit.diff.DiffEntry.ChangeType;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 
+import com.gitblit.Keys;
 import com.gitblit.models.PathModel.PathChangeModel;
 import com.gitblit.models.RefModel;
 import com.gitblit.models.RepositoryModel;
@@ -111,7 +112,14 @@ public class ComparePage extends RepositoryPage {
 			fromCommitId.setObject(startId);
 			toCommitId.setObject(endId);
 
-			final DiffOutput diff = DiffUtils.getDiff(r, fromCommit, toCommit, DiffOutputType.HTML);
+			final List<String> imageExtensions = app().settings().getStrings(Keys.web.imageExtensions);
+			final ImageDiffHandler handler = new ImageDiffHandler(getContextUrl(), repositoryName,
+					fromCommit.getName(), toCommit.getName(), imageExtensions);
+
+			final DiffOutput diff = DiffUtils.getDiff(r, fromCommit, toCommit, DiffOutputType.HTML, handler);
+			if (handler.getImgDiffCount() > 0) {
+				addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs
+			}
 
 			// add compare diffstat
 			int insertions = 0;
diff --git a/src/main/java/com/gitblit/wicket/pages/ImageDiffHandler.java b/src/main/java/com/gitblit/wicket/pages/ImageDiffHandler.java
new file mode 100644
index 00000000..52bf13b9
--- /dev/null
+++ b/src/main/java/com/gitblit/wicket/pages/ImageDiffHandler.java
@@ -0,0 +1,150 @@
+/*
+ * Copyright 2014 Tom <tw201207@gmail.com>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.gitblit.wicket.pages;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+
+import org.apache.wicket.protocol.http.WicketURLEncoder;
+import org.eclipse.jgit.diff.DiffEntry;
+import org.eclipse.jgit.diff.DiffEntry.Side;
+import org.jsoup.nodes.Element;
+
+import com.gitblit.servlet.RawServlet;
+import com.gitblit.utils.DiffUtils;
+import com.gitblit.utils.HtmlBuilder;
+
+/**
+ * A {@link DiffUtils.BinaryDiffHandler BinaryDiffHandler} for images.
+ *
+ * @author Tom <tw201207@gmail.com>
+ */
+public class ImageDiffHandler implements DiffUtils.BinaryDiffHandler {
+
+	private final String oldCommitId;
+	private final String newCommitId;
+	private final String repositoryName;
+	private final String baseUrl;
+	private final List<String> imageExtensions;
+
+	private int imgDiffCount = 0;
+
+	public ImageDiffHandler(final String baseUrl, final String repositoryName, final String oldCommitId,
+			final String newCommitId, final List<String> imageExtensions) {
+		this.baseUrl = baseUrl;
+		this.repositoryName = repositoryName;
+		this.oldCommitId = oldCommitId;
+		this.newCommitId = newCommitId;
+		this.imageExtensions = imageExtensions;
+	}
+
+	/** {@inheritDoc} */
+	@Override
+	public String renderBinaryDiff(DiffEntry diffEntry) {
+		switch (diffEntry.getChangeType()) {
+		case MODIFY:
+		case RENAME:
+		case COPY:
+			// TODO: for very small images such as icons, the slider doesn't really help. Two possible
+			// approaches: either upscale them for display (may show blurry upscaled images), or show
+			// them side by side (may still be too small to really make out the differences).
+			String oldUrl = getImageUrl(diffEntry, Side.OLD);
+			String newUrl = getImageUrl(diffEntry, Side.NEW);
+			if (oldUrl != null && newUrl != null) {
+				imgDiffCount++;
+				String id = "imgdiff" + imgDiffCount;
+				HtmlBuilder builder = new HtmlBuilder("div");
+				Element wrapper = builder.root().attr("class", "imgdiff-container").attr("id", "imgdiff-" + id);
+				Element container = wrapper.appendElement("div").attr("class", "imgdiff-ovr-slider").appendElement("div").attr("class", "imgdiff");
+				Element old = container.appendElement("div").attr("class", "imgdiff-left");
+				// style='max-width:640px;' is necessary for ensuring that the browser limits large images
+				// to some reasonable width, and to override the "img { max-width: 100%; }" from bootstrap.css,
+				// which would scale the left image to the width of its resizeable container, which isn't what
+				// we want here. Note that the max-width must be defined directly as inline style on the element,
+				// otherwise browsers ignore it if the image is larger, and we end up with an image display that
+				// is too wide.
+				// XXX: Maybe add a max-height, too, to limit portrait-oriented images to some reasonable height?
+				// (Like a 300x10000px image...)
+				old.appendElement("img").attr("class", "imgdiff-old").attr("id", id).attr("style", "max-width:640px;").attr("src", oldUrl);
+				container.appendElement("img").attr("class", "imgdiff").attr("style", "max-width:640px;").attr("src", newUrl);
+				wrapper.appendElement("br");
+				wrapper.appendElement("div").attr("class", "imgdiff-opa-container").appendElement("div").attr("class", "imgdiff-opa-slider");
+				return builder.toString();
+			}
+			break;
+		case ADD:
+			String url = getImageUrl(diffEntry, Side.NEW);
+			if (url != null) {
+				return new HtmlBuilder("img").root().attr("class", "diff-img").attr("src", url).toString();
+			}
+			break;
+		default:
+			break;
+		}
+		return null;
+	}
+
+	/** Returns the number of image diffs generated so far by this {@link ImageDiffHandler}. */
+	public int getImgDiffCount() {
+		return imgDiffCount;
+	}
+
+	/**
+	 * Constructs a URL that will fetch the designated resource in the git repository. The returned string will
+	 * contain the URL fully URL-escaped, but note that it may still contain unescaped ampersands, so the result
+	 * must still be run through HTML escaping if it is to be used in HTML.
+	 *
+	 * @return the URL to the image, if the given {@link DiffEntry} and {@link Side} refers to an image, or {@code null} otherwise.
+	 */
+	protected String getImageUrl(DiffEntry entry, Side side) {
+		String path = entry.getPath(side);
+		int i = path.lastIndexOf('.');
+		if (i > 0) {
+			String extension = path.substring(i + 1);
+			for (String ext : imageExtensions) {
+				if (ext.equalsIgnoreCase(extension)) {
+					String commitId = Side.NEW.equals(side) ? newCommitId : oldCommitId;
+					if (commitId != null) {
+						return RawServlet.asLink(baseUrl, urlencode(repositoryName), commitId, urlencode(path));
+					} else {
+						return null;
+					}
+				}
+			}
+		}
+		return null;
+	}
+
+	/**
+	 * Encode a URL component of a {@link RawServlet} URL in the special way that the servlet expects it. Note that
+	 * the %-encoding used does not encode '&amp;' or '&lt;'. Slashes are not encoded in the result.
+	 *
+	 * @param component
+	 *            to encode using %-encoding
+	 * @return the encoded component
+	 */
+	protected String urlencode(final String component) {
+		// RawServlet handles slashes itself. Note that only the PATH_INSTANCE fits the bill here: it encodes
+		// spaces as %20, and we just have to correct for encoded slashes. Java's standard URLEncoder would
+		// encode spaces as '+', and I don't know what effects that would have on other parts of GitBlit. It
+		// would also be wrong for path components (but fine for a query part), so we'd have to correct it, too.
+		//
+		// Actually, this should be done in RawServlet.asLink(). As it is now, this may be incorrect if that
+		// operation ever uses query parameters instead of paths, or if it is fixed to urlencode its path
+		// components. But I don't want to touch that static method in RawServlet.
+		return WicketURLEncoder.PATH_INSTANCE.encode(component, StandardCharsets.UTF_8.name()).replaceAll("%2[fF]", "/");
+	}
+}
diff --git a/src/main/java/com/gitblit/wicket/pages/scripts/imgdiff.js b/src/main/java/com/gitblit/wicket/pages/scripts/imgdiff.js
new file mode 100644
index 00000000..c98a05a4
--- /dev/null
+++ b/src/main/java/com/gitblit/wicket/pages/scripts/imgdiff.js
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2014 Tom <tw201207@gmail.com>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+(function($) {
+
+/**
+ * Sets up elem as a slider; returns an access object. Elem must be positioned!
+ * Note that the element may contain other elements; this is used for instance
+ * for the image diff overlay slider.
+ *
+ * The styling of the slider is to be done in CSS. Currently recognized options:
+ * - initial: <float> clipped to [0..1], default 0
+ * - handleClass: <string> to assign to the handle div element created.
+ * If no handleClass is specified, a very plain default style is assigned.
+ */
+function rangeSlider(elem, options) {
+	options = $.extend({ initial : 0 }, options || {});
+	options.initial = Math.min(1.0, Math.max(0, options.initial));
+	
+	var $elem = $(elem);
+	var $handle = $('<div></div>').css({ position: 'absolute', left: 0, cursor: 'ew-resize' });
+	var $root = $(document.documentElement);
+	var $doc = $(document);	
+	var lastRatio = options.initial;
+
+	/** Mousemove event handler to track the mouse and move the slider. Generates slider:pos events. */
+	function track(e) {
+		var pos = $elem.offset().left;
+		var width = $elem.innerWidth();
+		var handleWidth = $handle.outerWidth(false);
+		var range = width - handleWidth;
+		if (range <= 0) return;
+		var delta = Math.min(range, Math.max (0, e.pageX - pos - handleWidth / 2));
+		lastRatio = delta / range;
+		$handle.css('left', "" + (delta * 100 / width) + '%');
+		$elem.trigger('slider:pos', { ratio: lastRatio, handle: $handle[0] });
+	}
+
+	/** Mouseup event handler to stop mouse tracking. */
+	function end(e) {
+		$doc.off('mousemove', track);
+		$doc.off('mouseup', end);
+		$root.removeClass('no-select');
+	}
+
+    /** Snaps the slider to the given ratio and generates a slider:pos event with the new ratio. */
+	function setTo(ratio) {
+		var w = $elem.innerWidth();
+		if (w <= 0 || $elem.is(':hidden')) return;
+		lastRatio = Math.min( 1.0, Math.max(0, ratio));
+		$handle.css('left', "" + Math.max(0, 100 * (lastRatio * (w - $handle.outerWidth(false))) / w) + '%');
+		$elem.trigger('slider:pos', { ratio: lastRatio, handle: $handle[0] });
+	}
+	
+	/**
+	 * Moves the slider to the given ratio, clipped to [0..1], in duration milliseconds.
+	 * Generates slider:pos events during the animation. If duration <= 30, same as setTo.
+	 * Default duration is 500ms. If a callback is given, it's called once the animation
+	 * has completed.
+	 */
+	function moveTo(ratio, duration, callback) {
+		ratio = Math.min(1.0, Math.max(0, ratio));
+		if (ratio === lastRatio) {
+			if (typeof callback == 'function') callback();
+			return;
+		}
+		if (typeof duration == 'undefined') duration = 500;
+		if (duration <= 30) {
+			 // Cinema is 24 or 48 frames/sec, so 20-40ms per frame. Makes no sense to animate for such a short duration.
+			setTo(ratio);
+			if (typeof callback == 'function') callback();
+		} else {
+			var target = ratio * ($elem.innerWidth() - $handle.outerWidth(false));
+			if (ratio > lastRatio) target--; else target++;
+			$handle.stop().animate({left: target},
+				{ 'duration' : duration,
+				  'step' : function() {
+						lastRatio = Math.min(1.0, Math.max(0, $handle.position().left / ($elem.innerWidth() - $handle.outerWidth(false))));
+						$elem.trigger('slider:pos', { ratio : lastRatio, handle : $handle[0] });
+					},
+				  'complete' : function() { setTo(ratio); if (typeof callback == 'function') callback(); } // Ensure we have again a % value
+				}
+			);
+		}
+	}
+	
+	/**
+	 * As moveTo, but determines an appropriate duration in the range [0..maxDuration] on its own,
+	 * depending on the distance the handle would move. If no maxDuration is given it defaults
+	 * to 1500ms.
+	 */
+	function moveAuto(ratio, maxDuration, callback) {
+		if (typeof maxDuration == 'undefined') maxDuration = 1500;
+		var delta = ratio - lastRatio;
+		if (delta < 0) delta = -delta;
+		var speed = $elem.innerWidth() * delta * 2;
+		if (speed > maxDuration) speed = maxDuration;
+		moveTo(ratio, speed, callback);
+	}
+
+	/** Returns the current ratio. */
+	function getValue() {
+		return lastRatio;
+	}
+
+	$elem.append($handle);
+	if (options.handleClass) {
+		$handle.addClass(options.handleClass);
+	} else { // Provide a default style so that it is at least visible
+		$handle.css({ width: '10px', height: '10px', background: 'white', border: '1px solid black' });
+	}
+	if (options.initial) setTo(options.initial);
+
+	/** Install mousedown handler to start mouse tracking. */
+	$handle.on('mousedown', function(e) {
+		$root.addClass('no-select');
+		$doc.on('mousemove', track);
+		$doc.on('mouseup', end);
+		e.stopPropagation();
+		e.preventDefault();
+	});
+
+	return { setRatio: setTo, moveRatio: moveTo, 'moveAuto': moveAuto, getRatio: getValue, handle: $handle[0] };
+}
+
+function setup() {
+	$('.imgdiff-container').each(function() {
+		var $this = $(this);
+		var $overlaySlider = $this.find('.imgdiff-ovr-slider').first();
+		var $opacitySlider = $this.find('.imgdiff-opa-slider').first();
+		var overlayAccess = rangeSlider($overlaySlider, {handleClass: 'imgdiff-ovr-handle'});
+		var opacityAccess = rangeSlider($opacitySlider, {handleClass: 'imgdiff-opa-handle'});
+		var $img = $('#' + this.id.substr(this.id.indexOf('-')+1)); // Here we change opacity
+		var $div = $img.parent(); // This controls visibility: here we change width.
+		
+		$overlaySlider.on('slider:pos', function(e, data) {
+			var pos = $(data.handle).offset().left;
+			var imgLeft = $img.offset().left; // Global
+			var imgW = $img.outerWidth(true);
+			var imgOff = $img.position().left; // From left edge of $div
+			if (pos <= imgLeft) {
+				$div.width(0);
+			} else if (pos <= imgLeft + imgW) {
+				$div.width(pos - imgLeft + imgOff);
+			} else if ($div.width() < imgW + imgOff) {
+				$div.width(imgW + imgOff);
+			}
+		});
+		$overlaySlider.css('cursor', 'pointer');
+		$overlaySlider.on('mousedown', function(e) {
+			var newRatio = (e.pageX - $overlaySlider.offset().left) / $overlaySlider.innerWidth();
+			var oldRatio = overlayAccess.getRatio();
+			if (newRatio !== oldRatio) {
+				overlayAccess.moveAuto(newRatio);
+			}
+		});
+		$opacitySlider.on('slider:pos', function(e, data) {
+			if ($div.width() <= 0) overlayAccess.moveAuto(1.0); // Make old image visible in a nice way
+			$img.css('opacity', 1.0 - data.ratio);
+		});
+		$opacitySlider.css('cursor', 'pointer');
+		$opacitySlider.on('mousedown', function(e) {
+			var newRatio = (e.pageX - $opacitySlider.offset().left) / $opacitySlider.innerWidth();
+			var oldRatio = opacityAccess.getRatio();
+			if (newRatio !== oldRatio) {
+				if ($div.width() <= 0) {
+					overlayAccess.moveRatio(1.0, 500, function() {opacityAccess.moveAuto(newRatio);}); // Make old image visible in a nice way
+				} else {
+					opacityAccess.moveAuto(newRatio)
+				}
+			}
+			e.preventDefault();
+		});
+			
+	});
+}
+
+$(setup); // Run on jQuery's dom-ready
+
+})(jQuery);
\ No newline at end of file
diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css
index 1064231c..5a62de0b 100644
--- a/src/main/resources/gitblit.css
+++ b/src/main/resources/gitblit.css
@@ -1438,6 +1438,138 @@ div.diff > table {
 	color: #555;

 }

 

+/* Image diffs. */

+

+/* Set on body during mouse tracking. */

+.no-select {

+	-webkit-touch-callout:none;

+	-webkit-user-select:none;

+	-khtml-user-select:none;

+	-moz-user-select:none;

+	-ms-user-select:none;

+	user-select:none;

+}

+

+div.imgdiff-container {

+	padding: 10px;

+	background: #EEE;

+}

+

+div.imgdiff {

+	margin: 10px 20px;

+	position:relative;

+	display: inline-block;

+	/* Checkerboard background to reveal transparency. */

+    background-color: white;

+    background-image: linear-gradient(45deg, #DDD 25%, transparent 25%, transparent 75%, #DDD 75%, #DDD), linear-gradient(45deg, #DDD 25%, transparent 25%, transparent 75%, #DDD 75%, #DDD);

+    background-size:16px 16px;

+    background-position:0 0, 8px 8px;

+}

+

+div.imgdiff-left {

+	position: absolute;

+	top: 0;

+	bottom: 0;

+	left: 0;

+	width: 0;

+	max-width: 100%;

+	overflow: hidden;

+}

+

+img.imgdiff {

+	user-select: none;

+	border: 1px solid #0F0;

+}

+img.imgdiff-old {

+	user-select: none;

+	border: 1px solid #F00;

+}

+.imgdiff-opa-container {

+	width: 200px;

+	height: 4px;

+	margin: 12px 35px;

+	padding: 0;

+	position: relative;

+	border-left: 1px solid #888;

+	border-right: 1px solid #888;

+	background: linear-gradient(to bottom, #888, #EEE 50%, #888);

+}

+

+.imgdiff-opa-container:before {

+	content: '';

+	position: absolute;

+	left: -20px;

+	top: -4px;

+	width : 12px;

+	height: 12px;

+	background-image: radial-gradient(6px at 50% 50%, rgba(255, 255, 255, 255) 50%, rgba(255, 255, 255, 0) 6px);

+}

+

+.imgdiff-opa-container:after {

+	content: '';

+	position: absolute;

+	right: -20px;

+	top: -4px;

+	width : 12px;

+	height: 12px;

+	background-image: radial-gradient(6px at 50% 50%, #888, #888 1px, transparent 6px);

+}

+

+.imgdiff-opa-slider {

+	position:absolute;

+	top : 0;

+	left: -5px;

+	bottom: 0;

+	right: -5px;

+	text-align: left;

+}

+

+.imgdiff-opa-handle {

+	width: 10px;

+	height: 10px;

+	position: absolute;

+	top: -3px;

+	background-image: radial-gradient(5px at 50% 50%, #444, #888, transparent 5px);

+}

+

+.imgdiff-ovr-slider {

+	display: inline-block;

+	margin: 0;

+	padding: 0;

+	position: relative;

+	text-align: left;

+}

+

+.imgdiff-ovr-handle {

+	width : 2px;

+	height: 100%;

+	top: 0px;

+	background: linear-gradient(to right, #444, #FFF);

+}

+

+.imgdiff-ovr-handle:before {

+	content: '';

+	position: absolute;

+	right: -4px;

+	bottom: -5px;

+	width : 10px;

+	height: 10px;

+	background-image: radial-gradient(5px at 50% 50%, #444, #888, transparent 5px);

+}

+

+.imgdiff-ovr-handle:after {

+	content: '';

+	position: absolute;

+	right: -4px;

+	top: -5px;

+	width : 10px;

+	height: 10px;

+	/* border: 1px solid red; */

+	background-image: radial-gradient(5px at 50% 50%, #444, #888, transparent 5px);

+}

+

+/* End image diffs */

+

 td.changeType {

 	width: 15px;

 }