Issue 1233: "Expand 10 before" / "Expand 10 after" doesn't always work
Status:  Submitted
Owner:
Closed:  Oct 2013
Reported by marc...@google.com, Jan 11, 2012
Affected Version: (2.2.1-463-g40deada) 

I was excited to see the "expand 10 before/after" links for the common section, but when I went to try this feature, it didn't work for me.
Every time I click on either the "before", "common" or "after" links, all I get is one blank line.
This line will have a line number on the left hand side of the diff, but no code, while the right hand side diff doesn't even have a line number. Clicking again results in another copy of the blank line (same line number) being inserted.

Jan 11, 2012
#1 marc...@google.com
Oh, and it does this on both Chrome and Firefox (both on Linux)

Jan 11, 2012
Project Member #2 bklarson@gmail.com
Is it possible the file you are diffing when you see this behavior has over 9,000 lines?  There is a known bug where we don't work with really large files currently.

Failing that, there might be another issue if you are ignoring whitespace (on diff page, Differences->Preferences->Ignore Whitespace: anything but None).  Is that a possibility?

Failing that, are you able to reproduce this with any diffs on a publicly-available Gerrit instance?
Status: AwaitingInformation
Owner: bklarson@gmail.com
Labels: -Priority-Minor Priority-Major
Jan 11, 2012
#3 marc...@google.com
The files I tried had (much) less than 9000 lines, but I do have ignore whitespace turned on. Turning that off (setting it to "None" and then clicking "update") doesn't fix the problem though.

I couldn't find a public Gerrit instance that has this feature. If you can point me at one, I'd be happy to try.

Jan 11, 2012
Project Member #4 bklarson@gmail.com
gerrit-review.google.com is the only one I'm aware of..

Sorry for the trouble, and thanks for the help debugging!
Jan 11, 2012
Project Member #5 edwin.ke...@gmail.com
Not sure if this helps, but on https://gerrit-review.googlesource.com there is one change where this problem can be seen:

* make sure you are logged in
* goto https://gerrit-review.googlesource.com/#/c/17380/9/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProject.java
* compare patch set 8 and 9
* click on any link for expanding the skipped lines

Please note that the compared file was deleted in patch set 9. Unfortunately the formatting doesn't show this. The formatting problem was reported in issue 1222 [1].

[1] https://code.google.com/p/gerrit/issues/detail?id=1222
Jan 11, 2012
Project Member #7 bklarson@gmail.com
Yeah I read over issue 1222 when you reported it... I assume the skipped line oddity is a byproduct of the bad diff, and will be fixed when that diff issue gets sorted out.  But it is probably worth further investigation, I'll see if i can dig anything up related to issue 1222.

Edwin, are you ever on irc?  There have been lots of times where I've had questions that I was sure you could answer :)
Jan 11, 2012
#8 marc...@google.com
The expansion works for the change Edwin mentioned in comment #5.
I also tried https://gerrit-review.googlesource.com/#/c/30790/1/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Screen.java and it works there too.

Jan 20, 2012
#9 marc...@google.com
the bug status for this is still "AwaitingInformation". Do you still need more info from me?

Jan 23, 2012
Project Member #10 bklarson@gmail.com
I'm not sure that I'll be able to figure out what is going wrong until this can be seen on a public server... The patch which adds the expand lines feature is really simple - the server already sends the client the full files so they can be colored/marked up properly, we just display more of the file as requested.  I'm not sure what could be causing this.

If/when your internal server is upgraded to 2.2.2, let me know if you still see this problem.  If this server happens to be administrated by Shawn, you might ask him if he has any ideas if there are any custom changes on this server which could impact how this feature works.
Status: Accepted
Apr 30, 2012
#11 s...@imsand.li
I can reproduce this problem when Syntax Coloring is disabled. v2.3
Apr 30, 2012
#12 marc...@google.com
Yes! Turning on syntax coloring fixes the issue for me too (and for the changes that I said worked for me earlier, turning off syntax coloring makes expand before/after stop working)

Oct 26, 2012
Project Member #13 bklarson@gmail.com
 Issue 1629  has been merged into this issue.
Nov 3, 2012
#14 nemow...@gmail.com
In case more examples (from a public 2.4 instance) are needed: https://gerrit.wikimedia.org/r/#/c/14040/1/wmf-config/InitialiseSettings.php (and all changes to this file, it seems).
Dec 20, 2012
#15 fr...@benkstein.net
Hi,

I am seeing this now myself on Firefox 17 and 18 and also on a colleagues Internet Explorer (not sure which version).

When this occurs in Firefox there is also a record like this in the Error Console:

Timestamp: 12/20/2012 02:58:09 PM
Error: uncaught exception: Class$UmbrellaException: One or more exceptions caught, see full set in UmbrellaException#getCauses

I also tried looking at this in Firefox' Javascript debugger.  It catches the exception but I can't make sense of what I see.

Hope this helps,
Frank.


Apr 1, 2013
Project Member #16 edwin.ke...@gmail.com
 Issue 1851  has been merged into this issue.
Apr 2, 2013
#17 marc...@google.com
Still broken when syntax coloring is off. In Chrome I see similar exceptions as mentioned in comment #15 above:

Uncaught com.google.gwt.event.shared.UmbrellaException: Exception caught: Array index 0 out of range D2F9648551E6E53E4DC10731A8F39C35.cache.html:2146Uncaught com.google.gwt.event.shared.UmbrellaException: Exception caught: Array index 3428 out of range D2F9648551E6E53E4DC10731A8F39C35.cache.html:2146

Oct 2, 2013
Project Member #18 dougk....@gmail.com
A partial fix is available by https://gerrit-review.googlesource.com/50460

This prevents even displaying the "expand" links unless it's possible (i.e. the whole file has been sent to the browser).  This won't happen for extremely long files, still, so a more elegant solution is needed, but my hope is it  won't cause the JavaScript exception or display infinite blank lines.
Oct 7, 2013
#19 sop@google.com
(No comment was entered for this change.)
Status: Submitted