LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 30572 - Can't delete inline comment after last upgrade
Summary: Can't delete inline comment after last upgrade
Status: CONFIRMED
Alias: None
Product: Website
Classification: Unclassified
Component: Phabricator (show other bugs)
Version: unspecified
Hardware: PC All
: P normal
Assignee: Eric Liu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-30 00:48 PDT by Mehdi Amini
Modified: 2017-01-17 19:13 PST (History)
6 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mehdi Amini 2016-09-30 00:48:30 PDT
I can now only edit, but not delete comment. When I click on the trash it grays and stays like that forever.
Comment 1 Mehdi Amini 2016-09-30 10:38:43 PDT
I have to first edit the comment for this to happen apparently.
Comment 2 Eric Liu 2016-10-08 09:53:57 PDT
Hi Mehdi,

Do you mean unposted inline comments or comments to the patch?

I've tried deleting unposted inline comments by clicking the "trash" button, which always worked. I tested with Chrome browser; which browser and version are you using?

For comments to the patch, there is no "delete" button. The comment is deleted when you delete all the text, although the comment preview might not be updated immediately after you delete the text.

- Eric
Comment 3 Mehdi Amini 2016-10-08 12:58:22 PDT
I'm talking about unposted inline comments.

Have you tried first *editing* and *saving* it before trying to delete it?
Comment 4 Aaron Ballman 2016-11-11 15:45:44 PST
FWIW, I ran into this same issue. I tried editing the comment to be empty and saving, but it was not allowed. I tried editing the comment to not be empty and saving, then deleting. The save worked, but the delete still failed. Eventually, I edited the comment to complain about the phab bug and hit submit.

I have not had this issue with all comments I try to delete. It's only happened to me one time, and I'm not certain what steps I took to get me into the weird state.
Comment 5 Matthias Braun 2016-11-11 15:46:42 PST
The same just happened to me. I can reliably reproduce it by:

- Add inline comment to some line in the source, save it.
- Edit the comment, slightly changing the text.
- Press the remove button, they comment becomes greyed out but does not disappear and will be back once you reload the page.
Comment 6 Matthias Braun 2016-11-11 18:58:10 PST
Moved to Website/Phabricator (see https://llvm.org/PR30388)
Comment 7 Eric Liu 2016-11-12 08:29:36 PST
Thanks Matthias! I can finally reproduce this. I'll check if this is caused by my local Phab modification. Otherwise, I'll check if this is fixed in Phab upstream or report the issue there.
Comment 8 Justin Lebar (:jlebar) 2017-01-12 23:11:37 PST
Friendly ping?
Comment 9 Eric Liu 2017-01-13 08:53:36 PST
Sorry that I lost track of this issue.

This only happens in the production instance but not the testing instance (with the same revision and local changes), so I couldn't report the issue to Phabricator upstream with a reproducer. I tried hunting the bug myself but failed (Unfortunately, I am not a phabricator or php expert :(

I can try syncing to upstream head to see if this can be magically fixed.
Comment 10 Justin Lebar (:jlebar) 2017-01-13 12:33:00 PST
> This only happens in the production instance but not the testing instance

Well that's a bummer.  Maybe it's load/timing related.

> I can try syncing to upstream head to see if this can be magically fixed.

That would certainly be appreciated.  If nothing else maybe it will fix some of the other outstanding issues.
Comment 11 Eric Liu 2017-01-16 15:18:25 PST
Looks like the recent phabricator upgrade resolved this issue.

Could you please check if this is fixed for you? Thanks!
Comment 12 Justin Lebar (:jlebar) 2017-01-17 18:02:54 PST
I can't speak to the comment-deletion bug (I was never able to reproduce on-command, so if you were, I believe you when you say that it's better now).  

Unfortunately now I cannot select text inside of inline phab comments.  (Chrome 55, Ubuntu.)
Comment 13 Matthias Braun 2017-01-17 18:26:35 PST
The comment deletion bug is fixed.
@jlebar: I am not exactly sure what you mean, but selecting text in the inline comment box within the sourcecode and in the same text in that timeline view above the code with the mouse for copy&pasting work for me on Safari and chrome 55.0.2883.95 on macOS.
Comment 14 Justin Lebar (:jlebar) 2017-01-17 18:57:53 PST
(In reply to comment #13)
> @jlebar: I am not exactly sure what you mean, but selecting text in the
> inline comment box within the sourcecode and in the same text in that
> timeline view above the code with the mouse for copy&pasting work for me on
> Safari and chrome 55.0.2883.95 on macOS.

Yes, I think we're talking about the same thing.

STR for me: Load https://reviews.llvm.org/D28794, ctrl+f, search for "is there a reason".  There are two instances of this phrase in the page.

The first instance appears outside of the patch body.  I can highlight this fine.  The second instance appears inside of the patch body.  I am unable to highlight this text using my mouse cursor.
Comment 15 Matthias Braun 2017-01-17 19:11:28 PST
(In reply to comment #14)
> (In reply to comment #13)
> > @jlebar: I am not exactly sure what you mean, but selecting text in the
> > inline comment box within the sourcecode and in the same text in that
> > timeline view above the code with the mouse for copy&pasting work for me on
> > Safari and chrome 55.0.2883.95 on macOS.
> 
> Yes, I think we're talking about the same thing.
> 
> STR for me: Load https://reviews.llvm.org/D28794, ctrl+f, search for "is
> there a reason".  There are two instances of this phrase in the page.
> 
> The first instance appears outside of the patch body.  I can highlight this
> fine.  The second instance appears inside of the patch body.  I am unable to
> highlight this text using my mouse cursor.

Yes this works for me on macOS.
Comment 16 Aaron Ballman 2017-01-17 19:13:40 PST
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > @jlebar: I am not exactly sure what you mean, but selecting text in the
> > > inline comment box within the sourcecode and in the same text in that
> > > timeline view above the code with the mouse for copy&pasting work for me on
> > > Safari and chrome 55.0.2883.95 on macOS.
> > 
> > Yes, I think we're talking about the same thing.
> > 
> > STR for me: Load https://reviews.llvm.org/D28794, ctrl+f, search for "is
> > there a reason".  There are two instances of this phrase in the page.
> > 
> > The first instance appears outside of the patch body.  I can highlight this
> > fine.  The second instance appears inside of the patch body.  I am unable to
> > highlight this text using my mouse cursor.
> 
> Yes this works for me on macOS.

Also works for me on Windows 10 with Chrome 55.0.2883.87 m.