Page MenuHomePhabricator
Create Task
Maniphest T11662

Draft comments which reply to ghost inlines from lines which no longer exist can not be edited or deleted until the page is reloaded
Closed, ResolvedPublic
Actions

Description

Reproduction instructions:

  • Create a revision with long.txt, which has about 100 lines.
  • Add an inline comment near the end of the file, saving and then submitting it.
  • Delete about half the file locally, so the new file is shorter than the original file was.
  • Update the revision.
  • Reload the page so you see the most up-to-date version, then reply to the comment (which is now a ghost). Save the reply, but do not submit it yet.
  • Try to edit the reply (which is still a draft).

Expected behavior:

  • Editing (and deleting) work properly.

Actual behavior:

  • The JS gets tripped up because the reply is placed "on line 100", but the file now only has about 50 lines. Javascript console spews some noise and the click is ignored.

Workaround:

  • Reload the page first, then edit the inline comment.

Screenshot:

Screen Shot 2016-09-19 at 1.42.11 PM.png (1×1 px, 156 KB)

This is related to the issue discussed in T10563#164374.

Revisions and Commits

rP Phabricator
D17930rPdde63af1cc86 Fix a JS console warning when hovering over replies to ghosts on lines which no…

Related Objects

Event Timeline

epriestley created this task.Sep 19 2016, 8:43 PM
Herald added subscribers: chad, eadler. · View Herald TranscriptSep 19 2016, 8:43 PM
epriestley moved this task from Backlog to Inline Infra on the Differential board.Dec 29 2016, 4:31 PM
chad removed a project: Phacility.Jan 23 2017, 2:15 AM
epriestley added a comment.Jan 23 2017, 3:12 PM

I tagged this with Phacility because a Phacility customer was the original reporter. I think it's useful to keep track of bugs which impact customers, but maybe I should make a different tag?

epriestley added a comment.Jan 23 2017, 3:13 PM

I think it's useful to keep track of bugs which impact customers

Specifically, because I'm willing to spend more effort/resources solving problems experienced by customers than other users, and having this information may lead to a different outcome for the bug.

chad added a project: Phacility.Jan 23 2017, 3:13 PM

No worries. I was like... weird there is a random task in "Phacility", so I removed one, then I saw like 50 more... and stopped.

epriestley added a project: Customer Impact.Apr 12 2017, 3:58 PM
epriestley removed a project: Phacility.
epriestley moved this task from Backlog to Active Impact on the Customer Impact board.
epriestley added a project: Inline Comments.Apr 21 2017, 12:20 PM
epriestley moved this task from Backlog to Glitches on the Inline Comments board.Apr 21 2017, 12:23 PM
epriestley moved this task from Glitches to Editor on the Inline Comments board.Apr 24 2017, 10:33 AM
epriestley added a comment.May 17 2017, 3:32 PM

I can no longer reproduce the edit issue at HEAD.

I suspect the cluster of changes connected to T12616 (which generally improved workflow robustness here) also corrected this.

We still get a warning about highlighting; I'm going to fix that by just skipping the line highlighting because there isn't truly a valid range of lines to highlight.

epriestley added a revision: D17930: Fix a JS console warning when hovering over replies to ghosts on lines which no longer exist.May 17 2017, 3:37 PM
epriestley closed this task as Resolved by committing rPdde63af1cc86: Fix a JS console warning when hovering over replies to ghosts on lines which no….May 17 2017, 3:55 PM
epriestley added a commit: rPdde63af1cc86: Fix a JS console warning when hovering over replies to ghosts on lines which no….
Phabricator · Built by Phacility