Fix the comment dialog overwriting a user's comment on load.

Review Request #4048 — Created April 13, 2013 and submitted

Information

Review Board
master

Reviewers

Fix the comment dialog overwriting a user's comment on load.

The comment dialog ensures the comment is ready before display by
loading it. It's always done this. However, when latency is high,
there's a window of time where the user can type a comment and then have
it overwritten.

This is annoying, but it got worse with this rewrite. Calling ready()
would load the parent (which didn't used to happen for new comments),
and after that loaded, we'd set the text from the comment (which would
be blank by default). So, we'd end up clearing the comment the user
entered.

This change fixes both cases. We no longer load the text from the
server, since we already have it on page load. We trust that what we
have on page load is correct. It's possible this is wrong (if the user's
editing on two different tabs at once), but in the vast majority of
cases it should be completely safe.

We still load the parentObject. We just don't make immediate use of it.
This has a neat side-benefit, though. Comment saving for the first
comment will appear to be faster, since we won't be loading it at save
time. It will just happen on the initial comment, silently in the
background.

Along with this, I discovered that the Delete button wasn't always
visible when it should be. I've changed the logic to check isNew()
instead of loaded.
Added an artificial delay inside loading. I then tested new comments,
existing draft comments, and existing published comments.

In all cases, I was unable to reproduce the problem of my text getting
overwritten.

Unit tests pass.
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
      Ignored Files:
        reviewboard/static/rb/js/models/tests/commentEditorModelTests.js
        reviewboard/static/rb/js/models/commentEditorModel.js
        reviewboard/static/rb/js/models/baseResourceModel.js
    
    
  2. 
      
david
  1. Yay!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Loading...