rietveld-codereview / rietveld

Code Review, hosted on Google App Engine
https://codereview.appspot.com
Apache License 2.0
560 stars 152 forks source link

At publish/, clicking "Publish+Mail Comments" instead of "Publish all my drafts" will irrecoverably delete your draft comment #485

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Review some code, click Publish+Mail Comments
2. Write a wordy summary comment in "Message", get distracted
3. Click the "Publish+Mail Comments" hyperlink again. Lose summary comment.

What is the expected output? What do you see instead?

The link should not be there - it just points to the URL you are already at. 
Then, since it is the same URL, it triggers a reload rather than a navigation. 
This means you can't even press "Back" to retrieve the text you had in the text 
field.

What browser are you using?  What version? On what operating system?

Chrome Version 34.0.1847.76 beta on Mac

At what URL are you accessing Rietveld?

https://codereview.chromium.org

*** If you are a Google employee please say so *** yep :)

I had a poke around looking for a fix. I tried something hacky like the 
following, but got stuck on getting `make serve` to work. Happy to keep poking 
at it, but it's probably a really easy fix for someone who has a clue what 
they're doing :p

$ hg diff
diff -r 2526390e1e13 templates/issue_base.html
--- a/templates/issue_base.html Wed Mar 26 14:28:21 2014 -0700
+++ b/templates/issue_base.html Thu Mar 27 19:25:44 2014 +1100
@@ -65,13 +65,15 @@
               <span class="disabled">Can't Edit</span>
             {%endif%}
             <br/>
-            {%if user%}
-              <a class="novisit"
-                 href="{%url codereview.views.publish issue.key.id%}">
-                Publish+Mail Comments
-              </a> ('m')
-            {%else%}
-              <span class="disabled">Can't Publish+Mail</span>
+            {%if not form%}
+              {%if user%}
+                <a class="novisit"
+                   href="{%url codereview.views.publish issue.key.id%}">
+                  Publish+Mail Comments
+                </a> ('m')
+              {%else%}
+               <span class="disabled">Can't Publish+Mail</span>
+              {%endif%}
             {%endif%}
             {%if last_patchset and first_patch%}
               <br/>

Original issue reported on code.google.com by tapted@chromium.org on 27 Mar 2014 at 8:31

Attachments: