search

CandyShop / gerrit

Automatically exported from code.google.com/p/gerrit
Apache License 2.0
1 stars 0 forks source link

Left-side of side-by-side review for image review is broken #1984

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
************************************************************
***** NOTE: THIS BUG TRACKER IS FOR GERRIT CODE REVIEW *****
***** DO NOT SUBMIT BUGS FOR CHROME, ANDROID, INTERNAL *****
***** ISSUES WITH YOUR COMPANY'S GERRIT SETUP, ETC.    *****
***** THOSE ISSUE BELONG IN DIFFERENT ISSUE TRACKERS!  *****
************************************************************

Affected Version: 2.7-rc2-505-g7502a46

What steps will reproduce the problem?

1. A commit that touches an image (e.g. rename the image).
2. View it side-by-side
3. Observe

What is the expected behaviour?

See the old image on the left and the new on the right. Perhaps optimised to 
show only 1 image if binary files do not differ.

What do you see instead?

I see two (attempted to be) displayed images (two img tags). The left one leads 
to Not Found, the second displays the image correctly.

Please provide any additional information below.

Summary of diff:
 diff --git a/lib/resources/images/tipsy.png b/repo/resources/images/tipsy.png  
 similarity index 100%  
 rename from lib/resources/images/tipsy.png 
 rename to repo/resources/images/tipsy.png
 Binary files differ

 img[src] of left side:
 https://gerrit.wikimedia.org/r/cat/70115%2C6%2Crepo/resources/images/tipsy.png%5E1
 Leads to "Not Found"

 img[src] of right side:
 https://gerrit.wikimedia.org/r/cat/70115%2C6%2Crepo/resources/images/tipsy.png%5E0
 Leads to image in question.

Original issue reported on code.google.com by krinklemail@gmail.com on 30 Jun 2013 at 5:51

Attachments:

GoogleCodeExporter commented 9 years ago 
Note that for files, when similarity is 100%, it simply shows no diff at all. 
That'd be fine for images, too.

Original comment by krinklemail@gmail.com on 30 Jun 2013 at 5:53

GoogleCodeExporter commented 9 years ago

Original comment by dborowitz@google.com on 2 Jul 2013 at 5:12

GoogleCodeExporter commented 9 years ago 
This only affects renames, and it appears this code (ca. CatServlet.java:345) 
hasn't changed in a while. Can you confirm that this is a regression in 2.6?

Original comment by dborowitz@google.com on 16 Jul 2013 at 5:40

GoogleCodeExporter commented 9 years ago 
Oops, don't know where I got that line number, more like l. 197.

Original comment by dborowitz@google.com on 16 Jul 2013 at 5:41

GoogleCodeExporter commented 9 years ago 
Confirmed that this bug exists in 2.5.6. Definitely still a bug, but taking it 
off the Blocking-2.7 list since it's not a regression in 2.6 or 2.7.

Original comment by dborowitz@google.com on 16 Jul 2013 at 5:49