My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 735: displaying the unified diff for a newly added binary file fails
7 people starred this issue and may be notified of changes. Back to list
Status:  Released
Owner:  ----
Closed:  Aug 2012


Sign in to add a comment
 
Project Member Reported by edwin.ke...@gmail.com, Sep 23, 2010
Affected Version: 2.1.5

What steps will reproduce the problem?
1. Push a change for review that contains a new binary file
2. Open this change in Gerrit WebUI
3. Click on the binary file to open the unified diff for it

What is the expected output? What do you see instead?
Expected is that the unified diff for the newly added binary file is shown (or 
something else which makes sense). What happens is nothing.

If a reviewer wants to review the change by skipping through the file diffs (by 
navigating to the next/previous changed file by clicking on the link to the 
next/previous changed file) this problem also occurs. If the next/previous file 
is a newly added binary file nothing happens when the reviewer clicks on the 
link.

Please provide any additional information below.
On client side the following exception occurs:
00:01:29,562 [ERROR] Uncaught exception escaped
java.lang.ArrayIndexOutOfBoundsException: Array index out of range: 0
	at com.google.gerrit.prettify.common.SparseFileContent.get(SparseFileContent.java:74)
	at com.google.gerrit.prettify.common.PrettyFormatter.colorLineEdits(PrettyFormatter.java:377)
	at com.google.gerrit.prettify.common.PrettyFormatter.toHTML(PrettyFormatter.java:315)
	at com.google.gerrit.prettify.common.PrettyFormatter.format(PrettyFormatter.java:129)
	at com.google.gerrit.common.data.PatchScript.getSparseHtmlFileB(PatchScript.java:183)
	at com.google.gerrit.client.patches.UnifiedDiffTable.render(UnifiedDiffTable.java:90)
	at com.google.gerrit.client.patches.AbstractPatchContentTable.display(AbstractPatchContentTable.java:163)
	at com.google.gerrit.client.patches.PatchScreen.onResult(PatchScreen.java:435)
	at com.google.gerrit.client.patches.PatchScreen.access$3(PatchScreen.java:385)
	at com.google.gerrit.client.patches.PatchScreen$8.preDisplay(PatchScreen.java:371)
	at com.google.gerrit.client.patches.PatchScreen$8.preDisplay(PatchScreen.java:1)
	at com.google.gerrit.client.rpc.ScreenLoadCallback.onSuccess(ScreenLoadCallback.java:32)
	at com.google.gwtjsonrpc.client.JsonUtil.invoke(JsonUtil.java:136)
	...

I've spent some time to investigate this exception and identified the commit 
since when this exception is occuring:
bd000eb07cf5ad7eea6f1bf666329ce52891869d - "Honor user's syntax coloring 
preference in unified view"
The corresponding change in Gerrit can be found here [1].
It was done on the 'stable' branch and was merged back into the master branch 
with the commit 6001b80cc7b63740585feef81be63b9b29b0bb49.
The change does not directly introduce a bug but causes syntax highlighting for 
unified diffs. The algorithm which does the syntax highlighting seems to have a 
problem if a binary file was added.

From the symptoms this problem looks similar to issue 731 [2]. However since 
731 does not deal with binary files it might be something different. 
Unfortunately I can't reproduce 731 locally so I don't have a stacktrace for it.

[1] https://review.source.android.com/16637
[2] https://code.google.com/p/gerrit/issues/detail?id=731
Aug 8, 2012
Project Member #2 edwin.ke...@gmail.com
I believe this is fixed by [1].

[1] https://gerrit-review.googlesource.com/37261
Aug 9, 2012
#3 sop@google.com
(No comment was entered for this change.)
Status: Submitted
Labels: FixedIn-2.5
Oct 31, 2012
Project Member #4 edwin.ke...@gmail.com
(No comment was entered for this change.)
Status: Released
Dec 3, 2012
Project Member #5 edwin.ke...@gmail.com
 Issue 1697  has been merged into this issue.
Sign in to add a comment

Powered by Google Project Hosting