2011-02-09 Ojan Vafai <ojan@chromium.org>
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Feb 2011 05:12:02 +0000 (05:12 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Feb 2011 05:12:02 +0000 (05:12 +0000)
        Reviewed by Adam Barth.

        only erase draft comments after publish is successful
        https://bugs.webkit.org/show_bug.cgi?id=54163

        If there is a conflict or 500, then draft comments will survive.
        One drawback here is that the form post is now to the iframe,
        so to break out of the iframe we redirect to the bug page, which
        loses the information of who the email was sent to.

        Once WebKit supports seamless iframes we should be able to avoid
        the redirect.

        * code-review.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@78310 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/bugs.webkit.org/ChangeLog
Websites/bugs.webkit.org/code-review.js

index 0977cd611de4a44944c141ac740c17137e2893ed..bc27b87ac00ea5da61932fdf3ca7055d56fc03e6 100644 (file)
@@ -1,3 +1,20 @@
+2011-02-09  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        only erase draft comments after publish is successful
+        https://bugs.webkit.org/show_bug.cgi?id=54163
+
+        If there is a conflict or 500, then draft comments will survive.
+        One drawback here is that the form post is now to the iframe,
+        so to break out of the iframe we redirect to the bug page, which
+        loses the information of who the email was sent to.
+
+        Once WebKit supports seamless iframes we should be able to avoid
+        the redirect.
+
+        * code-review.js:
+
 2011-02-09  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Adam Barth.
index 8b035a3e2b48c0eaeaf2944d7071a8d6f5490f2e..d0b763d10d753b973ec17a1601e8fcbe2acb5ef2 100644 (file)
@@ -1066,8 +1066,28 @@ var CODE_REVIEW_UNITTEST;
   });
 
   function handleReviewFormLoad() {
-    var form = $('#reviewform').contents().find('form')[0];
-    form.addEventListener('submit', eraseDraftComments);
+    var review_form_contents = $('#reviewform').contents();
+    if (review_form_contents[0].querySelector('#form-controls #flags')) {
+      // This is the intial load of the review form iframe.
+      var form = review_form_contents.find('form')[0];
+      form.addEventListener('submit', eraseDraftComments);
+      form.target = '';
+      return;
+    }
+
+    // Review form iframe have the publish button has been pressed.
+    var email_sent_to = review_form_contents[0].querySelector('#bugzilla-body dl');
+    // If the email_send_to DL is not in the tree that means the publish failed for some reason,
+    // e.g., you're not logged in. Show the comment form to allow you to login.
+    if (!email_sent_to) {
+      showCommentForm();
+      return;
+    }
+
+    eraseDraftComments();
+    // FIXME: Once WebKit supports seamless iframes, we can just make the review-form
+    // iframe fill the page instead of redirecting back to the bug.
+    window.location.replace($('#toolbar .bugLink a').attr('href'));
   }
   
   function eraseDraftComments() {
@@ -1583,14 +1603,17 @@ var CODE_REVIEW_UNITTEST;
     });
   }
 
+  function showCommentForm() {
+    $('#comment_form').removeClass('inactive');
+  }
+
   $('#preview_comments').live('click', function() {
     fillInReviewForm();
-    $('#comment_form').removeClass('inactive');
+    showCommentForm();
   });
 
   $('#post_comments').live('click', function() {
     fillInReviewForm();
-    eraseDraftComments();
     $('#reviewform').contents().find('form').submit();
   });
 })();