Page MenuHomePhabricator

List of cases where ghost comments end up somewhere confusing or unexpected
Open, NormalPublic

Assigned To
Authored By
epriestley
Apr 21 2015, 12:46 PM
Referenced Files
F846957: Screen Shot 2015-09-28 at 2.25.44 PM.png
Sep 28 2015, 9:28 PM
F846950: Screen Shot 2015-09-28 at 2.25.21 PM.png
Sep 28 2015, 9:28 PM
F846953: Screen Shot 2015-09-28 at 2.25.33 PM.png
Sep 28 2015, 9:28 PM
F378072: samepage.png
Apr 21 2015, 8:01 PM
F378030: expdn.png
Apr 21 2015, 7:22 PM
F377791: move.png
Apr 21 2015, 3:19 PM
F377741: noreply.png
Apr 21 2015, 2:09 PM
F377707: missing.png
Apr 21 2015, 1:24 PM
Tokens
"The World Burns" token, awarded by chad.

Description

(Resolved by D12489) Old comments on the right-hand side port forward to the right-hand side, even if the left-hand side would be more appropriate.

  • Create diff 1, leave comments on the right-hand side.
  • Create diff 2, leave comments on the right-hand side.
  • Create diff 3, leave comments on the right-hand side.
  • Diff 2 vs 3.
  • Comments on diff 1 are shown on the right, but showing them on the left makes more sense.
  • Diff 1 vs 2.
  • Comments on diff 3 are shown on the right. This is desirable.

comment.png (440×1 px, 51 KB)


(Resolved by D12491) If a file is shortened (for example, it is mostly deleted), comments do not port forward into it.

  • Create diff 1, leave comments on the right-hand side in the middle/bottom of the file.
  • Delete almost all of the file.
  • Create diff 2.
  • View diff 2 vs base.
  • Expect to see comments at the end of right-hand side (maybe with annotations).
  • Actually see nothing.

missing.png (935×1 px, 165 KB)


(Resolved by D12492) Clicking "Reply" on comments which have been ported forward or backward does not work.

noreply.png (225×822 px, 18 KB)


(Resolved by D12493) Comments do not port forward or backward across file moves, but could reasonably be expected to.

  • Create diff 1, leave comments on "A".
  • Rename "A" to "B".
  • Create diff 2.
  • View diff 2 vs base.
  • Expect to see comments port forward.
  • Actually see nothing.
  • Add comments to "B".
  • View diff 1 vs base.
  • Expect to see comments port backward.
  • Actually see nothing.

move.png (618×1 px, 100 KB)


(Resolved by D12495) Ghost inlines do not expand changesets on very large revisions.

  • Create diff 1.
  • Add inlines.
  • Update to diff 2, affecting more than 100 files.
  • View diff 2 vs base.
  • Expect files with comments from diff 1 to auto-expand.
  • Actually, files do not auto-expand.

expdn.png (713×1 px, 74 KB)


(Resolved by D12497) Ghostly inlines visible on the current screen are marked as appearing on other diffs and link to other diffs in the timeline view, when they could reasonably link to some anchor on the same page.

  • Create diff 1.
  • Add comments.
  • Upload diff 2.
  • View diff 2 vs base.
  • Expect timeline view to show that the inlines are visible on the current page.
  • Actually see little arrow and link elsewhere.

samepage.png (584×1 px, 65 KB)


Ghostly comments may show up twice in the same diff-of-diffs.

  • Create diff 1 with file "A".
  • Create diff 2, moving "A" to "B".
  • View diff 2 vs diff 1.
  • Comments on either "A" or "B" show on both files.

Comment not porting here:

https://secure.phabricator.com/D13036?vs=on&id=31479&whitespace=ignore-most#136453

Might be the "move to last visible line" code interacting with the "port across rebases" code.


Ghost comments may port oddly in new files with removed lines? Repro case is:

  • Create a file with "a..z".
  • Remove a couple lines, update.
  • Add a comment somewhere near the bottom.
  • When viewed Diff 2 vs base, comment is not adjusted for removed lines.

Diff 1 with comment:

Screen Shot 2015-09-28 at 2.25.21 PM.png (733×1 px, 45 KB)

Diff 2 vs diff 1:

Screen Shot 2015-09-28 at 2.25.33 PM.png (732×1 px, 51 KB)

Diff 2 vs base (comment position is wrong):

Screen Shot 2015-09-28 at 2.25.44 PM.png (701×1 px, 44 KB)

Event Timeline

epriestley claimed this task.
epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a subscriber: avivey.

@avivey, added your case from D13036 above.

It's possible, but I'd guess they're probably not related if I had to.

The stuff here is almost entirely server-side and I'd guess the issue there is in the JS on the client somewhere, although I could be mistaken.

eadler added a project: Restricted Project.Jul 7 2016, 5:10 PM