Page MenuHomePhabricator

Gerrit's new side-by-side diff screen sometimes cuts off the last few characters of a line
Closed, ResolvedPublic

Description

See https://bugs.chromium.org/p/gerrit/issues/detail?id=4292 for upstream bug report. Repro case in our test instance is http://gerrit-test.wmflabs.org/gerrit/#/c/32/2/tests/fixtures/layout-cloner.yaml . Make sure you're in side-by-side mode, and make the window narrow enough that line 7 on the right-hand side doesn't fit. The ); at the end will become inaccessible to the scroll bars.

Event Timeline

Catrope created this task.Sep 1 2016, 11:21 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 1 2016, 11:21 PM
Catrope moved this task from Backlog to Reported Upstream on the Upstream board.Sep 1 2016, 11:22 PM

This seems to be fixed in polygerrit. Gerrit's new ui. Seee https://go-review.googlesource.com/?polygerrit=1 and https://go-review.googlesource.com/#/c/21732/4/src/internal/trace/parser.go

It seems to be CodeMirror that is the problem.

Paladox added a comment.EditedSep 5 2016, 3:18 PM

We will need to manually patch gerrit with the change, but once we do that, we can send it upstream for them to review :)

This is https://github.com/gerrit-review/gerrit/blob/7f4c48f37b9a8970ae2f8c8dc881f603ec9e707f/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java#L254 what needs patching and I will need to get approval from @demon if we can do this please? Since we can send it upstream for reviews after :)

Paladox added a subscriber: demon.Sep 5 2016, 3:19 PM

Ok I have built the deb but it is gerrit 2.13, I will need @demon to pull in upstream 2.12 into our repo here for me to submit our custom patch for gerrit 2.12 :)

@demon can we do a small customisation change to gerrit 2.12, to switch this default pref on by default please?

Paladox moved this task from Reported Upstream to Patch proposed upstream on the Upstream board.EditedSep 5 2016, 10:58 PM

Partially fixed in https://gerrit-review.googlesource.com/#/c/85340/ we can create our own custom patch to set it true for line wrapping

Paladox moved this task from Bugs & stuff to Maybe fixed? on the Gerrit board.
Paladox added a comment.EditedSep 23 2016, 10:01 AM

This is fixed in gerrit 2.12.5 which has been released now.

Aklapper triaged this task as Low priority.Sep 23 2016, 2:15 PM
demon closed this task as Resolved.Nov 15 2016, 6:26 PM
demon claimed this task.
demon closed subtask T143089: Update gerrit to 2.12.5 as Resolved.