Last modified: 2014-11-19 23:41:27 UTC
Environment- beta. 1> In VE, insert a comment. Click done. 2> click to undo while the comment is still highlighted. Notice the label on the Comment first changes to read “Comment” from the actual comment text, before removing it. See screencast.
http://youtu.be/ws3ObsyZijc
Confirmed in Beta(test2 and production do not have that issue).
This seems to be because the first undo you make only clears the comment -text- change. The second undo actually removes the comment node itself. This makes sense since you make the comment node and it goes in the document first, then later insert some text into it. ContextItemWidget#getDescription falls back to the tool's title if the text was falsey (so, an empty string), which is why you see 'Comment' rather than an empty box. This also seems reasonable. We should probably be removing an empty comment node from the document if it's left there due to undo... Currently this only appears to be done if you have an open comment inspector with no text and then click outside it (see CommentInspector#getTeardownProcess).
(In reply to Alex Monk from comment #3) > This seems to be because the first undo you make only clears the comment > -text- change. The second undo actually removes the comment node itself. > This makes sense since you make the comment node and it goes in the document > first, then later insert some text into it. > ContextItemWidget#getDescription falls back to the tool's title if the text > was falsey (so, an empty string), which is why you see 'Comment' rather than > an empty box. This also seems reasonable. > > We should probably be removing an empty comment node from the document if > it's left there due to undo... Currently this only appears to be done if you > have an open comment inspector with no text and then click outside it (see > CommentInspector#getTeardownProcess). This sounds like it was caused by Ed's "staging empty comment node" change. That code should ensure that the creation of the comment node and setting the value are one entry in the undo stack.
Change 173808 had a related patch set uploaded by Esanders: Make changes to comment node before apply staging stack https://gerrit.wikimedia.org/r/173808
Change 173808 merged by jenkins-bot: Make changes to comment node before apply staging stack https://gerrit.wikimedia.org/r/173808
Verified the fix in beta.
Verified the fix in test2.
