Bug 760989: Splinter should not require login

3 files changed, 115 insertions, 95 deletions

diff --git a/extensions/Splinter/Extension.pm b/extensions/Splinter/Extension.pm
index eef9e89af..c383297c4 100644
--- a/extensions/Splinter/Extension.pm
+++ b/extensions/Splinter/Extension.pm
@@ -33,8 +33,7 @@ sub page_before_template {
     my ($vars, $page) = @$args{qw(vars page_id)};
 
     if ($page eq 'splinter.html') {
-        # Login is required for performing a review
-        my $user = Bugzilla->login(LOGIN_REQUIRED);
+        my $user = Bugzilla->user;
 
         # We can either provide just a bug id to see a list
         # of prior reviews by the user, or just an attachment
@@ -54,7 +53,8 @@ sub page_before_template {
 
             # Check to see if the user can see the bug this attachment is connected to.
             Bugzilla::Bug->check($attachment->bug_id);
-            if ($attachment->isprivate && $user->id != $attachment->attacher->id
+            if ($attachment->isprivate
+                && $user->id != $attachment->attacher->id
                 && !$user->is_insider)
             {
                 ThrowUserError('auth_failure', {action => 'access',
diff --git a/extensions/Splinter/template/en/default/pages/splinter.html.tmpl b/extensions/Splinter/template/en/default/pages/splinter.html.tmpl
index fa6fd8316..80c0a314e 100644
--- a/extensions/Splinter/template/en/default/pages/splinter.html.tmpl
+++ b/extensions/Splinter/template/en/default/pages/splinter.html.tmpl
@@ -49,6 +49,7 @@
   Splinter.configHaveExtension = true;
   Splinter.configHelp = '[% urlbase FILTER js %]page.cgi?id=splinter/help.html';
   Splinter.configNote = '';
+  Splinter.readOnly = [% user.id FILTER none %] == 0;
 
   Splinter.configAttachmentStatuses = [
   [% FOREACH status = attachment_statuses %]
@@ -115,8 +116,10 @@
 <![endif]-->
 
 <div id="helpful-links">
-  <a id="allReviewsLink" href="[% Bugzilla.splinter_review_base FILTER none %]">
-    [reviews]</a>
+  [% IF user.id %]
+    <a id="allReviewsLink" href="[% Bugzilla.splinter_review_base FILTER none %]">
+      [reviews]</a>
+  [% END %]
   <a id='helpLink' target='splinterHelp' 
      href="[% urlbase FILTER none %]page.cgi?id=splinter/help.html">
     [help]</a>
@@ -193,61 +196,67 @@
       (Restored from draft; last edited <span id="restoredLastModified"></span>)
     </span>
   </div>
-  <div>
-    <div id="myCommentFrame">
-      <textarea id="myComment"></textarea>
-      <div id="emptyCommentNotice">&lt;Overall Comment&gt;</div>
-    </div>
-    <div id="myPatchComments"></div>
-    <form id="publish" method="post" action="attachment.cgi" onsubmit="normalizeComments();">
-      <input type="hidden" id="publish_token" name="token" value="">
-      <input type="hidden" id="publish_action" name="action" value="update">
-      <input type="hidden" id="publish_review" name="comment" value="">
-      <input type="hidden" id="publish_attach_id" name="id" value="">
-      <input type="hidden" id="publish_attach_desc" name="description" value="">
-      <input type="hidden" id="publish_attach_filename" name="filename" value="">
-      <input type="hidden" id="publish_attach_contenttype" name="contenttypeentry" value="">
-      <input type="hidden" id="publish_attach_ispatch" name="ispatch" value="">
-      <input type="hidden" id="publish_attach_isobsolete" name="isobsolete" value="">
-      <input type="hidden" id="publish_attach_isprivate" name="isprivate" value="">
-      <div id="attachment_flags">
-        [% any_flags_requesteeble = 0 %]
-        [% FOREACH flag_type = flag_types %]
-          [% NEXT UNLESS flag_type.is_active %]
-          [% SET any_flags_requesteeble = 1 IF flag_type.is_requestable && flag_type.is_requesteeble %]
-        [% END %]
-        [% IF flag_types.size > 0 %]
-          [% PROCESS "flag/list.html.tmpl" bug_id = bug_id
-                                           attach_id = attach_d
-                                           flag_types = flag_types
-                                           read_only_flags = !can_edit
-                                           any_flags_requesteeble = any_flags_requesteeble
-           %]
-        [% END %]
-        <script>
+  [% IF user.id %]
+    <div>
+      <div id="myCommentFrame">
+        <textarea id="myComment"></textarea>
+        <div id="emptyCommentNotice">&lt;Overall Comment&gt;</div>
+      </div>
+      <div id="myPatchComments"></div>
+      <form id="publish" method="post" action="attachment.cgi" onsubmit="normalizeComments();">
+        <input type="hidden" id="publish_token" name="token" value="">
+        <input type="hidden" id="publish_action" name="action" value="update">
+        <input type="hidden" id="publish_review" name="comment" value="">
+        <input type="hidden" id="publish_attach_id" name="id" value="">
+        <input type="hidden" id="publish_attach_desc" name="description" value="">
+        <input type="hidden" id="publish_attach_filename" name="filename" value="">
+        <input type="hidden" id="publish_attach_contenttype" name="contenttypeentry" value="">
+        <input type="hidden" id="publish_attach_ispatch" name="ispatch" value="">
+        <input type="hidden" id="publish_attach_isobsolete" name="isobsolete" value="">
+        <input type="hidden" id="publish_attach_isprivate" name="isprivate" value="">
+        <div id="attachment_flags">
+          [% any_flags_requesteeble = 0 %]
           [% FOREACH flag_type = flag_types %]
             [% NEXT UNLESS flag_type.is_active %]
-            Event.addListener('flag_type-[% flag_type.id FILTER js %]', 'change', 
-                              function() { Splinter.flagChanged = 1;        
-                                           Splinter.queueUpdateHaveDraft(); });
-            [% FOREACH flag = flag_type.flags %]
-              Event.addListener('flag-[% flag.id FILTER js %]', 'change', 
+            [% SET any_flags_requesteeble = 1 IF flag_type.is_requestable && flag_type.is_requesteeble %]
+          [% END %]
+          [% IF flag_types.size > 0 %]
+            [% PROCESS "flag/list.html.tmpl" bug_id = bug_id
+                                             attach_id = attach_d
+                                             flag_types = flag_types
+                                             read_only_flags = !can_edit
+                                             any_flags_requesteeble = any_flags_requesteeble
+            %]
+          [% END %]
+          <script>
+            [% FOREACH flag_type = flag_types %]
+              [% NEXT UNLESS flag_type.is_active %]
+              Event.addListener('flag_type-[% flag_type.id FILTER js %]', 'change',
                                 function() { Splinter.flagChanged = 1;
                                              Splinter.queueUpdateHaveDraft(); });
+              [% FOREACH flag = flag_type.flags %]
+                Event.addListener('flag-[% flag.id FILTER js %]', 'change',
+                                  function() { Splinter.flagChanged = 1;
+                                               Splinter.queueUpdateHaveDraft(); });
+              [% END %]
             [% END %]
-          [% END %]
-        </script>
+          </script>
+        </div>
+      </form>
+      <div id="buttonBox">
+        <span id="attachmentStatusSpan">Patch Status:
+          <select id="attachmentStatus"> </select>
+        </span>
+        <input id="publishButton" type="button" value="Publish" />
+        <input id="cancelButton" type="button" value="Cancel" />
       </div>
-    </form>
-    <div id="buttonBox">
-      <span id="attachmentStatusSpan">Patch Status:
-        <select id="attachmentStatus"> </select>
-      </span>
-      <input id="publishButton" type="button" value="Publish" />
-      <input id="cancelButton" type="button" value="Cancel" />
+      <div class="clear"></div>
     </div>
-    <div class="clear"></div>
-  </div>
+  [% ELSE %]
+    <div>
+      You must be logged in to review patches.
+    </div>
+  [% END %]
   <div id="oldReviews" style="display: none;">
     <div class="review-title">
       Previous Reviews
diff --git a/extensions/Splinter/web/splinter.js b/extensions/Splinter/web/splinter.js
index cfe7f3e1f..fae0e20fb 100644
--- a/extensions/Splinter/web/splinter.js
+++ b/extensions/Splinter/web/splinter.js
@@ -1388,6 +1388,10 @@ Splinter.discardReview = function () {
 };
 
 Splinter.haveDraft = function () {
+    if (Splinter.readOnly) {
+        return false;
+    }
+
     if (Splinter.theAttachment.status && Dom.get('attachmentStatus').value != Splinter.theAttachment.status) {
         return true;
     }
@@ -1966,7 +1970,7 @@ Splinter.appendPatchHunk = function (file, hunk, tableType, includeComments, cli
             }
         }
 
-        if (clickable) {
+        if (!Splinter.readOnly && clickable) {
             Splinter.domCache.data(tr).patchFile = file;
             Splinter.domCache.data(tr).patchLocation = loc;
             Event.addListener(tr, 'dblclick', Splinter.onRowDblClick);
@@ -2026,28 +2030,30 @@ Splinter.addPatchFile = function (file) {
     fileLabelStatus.appendChild(document.createTextNode(statusString));
     fileLabelStatus.appendTo(fileLabel);
 
-    var fileReviewed = new Element(document.createElement('span'));
-    Dom.addClass(fileReviewed, 'file-review');
-    Dom.setAttribute(fileReviewed, 'title', 'Indicates that a review has been completed for this file. ' +
-                                            'This is for personal tracking purposes only and has no effect ' +
-                                            'on the published review.');
-    fileReviewed.appendTo(fileLabel);
-
-    var fileReviewedInput = new Element(document.createElement('input'));
-    Dom.setAttribute(fileReviewedInput, 'type', 'checkbox');
-    Dom.setAttribute(fileReviewedInput, 'id', 'file-review-checkbox-' + encodeURIComponent(file.filename));
-    Dom.setAttribute(fileReviewedInput, 'onchange', "Splinter.toggleFileReviewed('" +
-                                                    encodeURIComponent(file.filename) + "');");
-    if (file.fileReviewed) {
-        Dom.setAttribute(fileReviewedInput, 'checked', 'true');
-    }
-    fileReviewedInput.appendTo(fileReviewed);
-
-    var fileReviewedLabel = new Element(document.createElement('label'));
-    Dom.addClass(fileReviewedLabel, 'file-review-label')
-    Dom.setAttribute(fileReviewedLabel, 'for', 'file-review-checkbox-' + encodeURIComponent(file.filename));
-    fileReviewedLabel.appendChild(document.createTextNode(' Reviewed'));
-    fileReviewedLabel.appendTo(fileReviewed);
+    if (!Splinter.readOnly) {
+        var fileReviewed = new Element(document.createElement('span'));
+        Dom.addClass(fileReviewed, 'file-review');
+        Dom.setAttribute(fileReviewed, 'title', 'Indicates that a review has been completed for this file. ' +
+                                                'This is for personal tracking purposes only and has no effect ' +
+                                                'on the published review.');
+        fileReviewed.appendTo(fileLabel);
+
+        var fileReviewedInput = new Element(document.createElement('input'));
+        Dom.setAttribute(fileReviewedInput, 'type', 'checkbox');
+        Dom.setAttribute(fileReviewedInput, 'id', 'file-review-checkbox-' + encodeURIComponent(file.filename));
+        Dom.setAttribute(fileReviewedInput, 'onchange', "Splinter.toggleFileReviewed('" +
+                                                        encodeURIComponent(file.filename) + "');");
+        if (file.fileReviewed) {
+            Dom.setAttribute(fileReviewedInput, 'checked', 'true');
+        }
+        fileReviewedInput.appendTo(fileReviewed);
+
+        var fileReviewedLabel = new Element(document.createElement('label'));
+        Dom.addClass(fileReviewedLabel, 'file-review-label')
+        Dom.setAttribute(fileReviewedLabel, 'for', 'file-review-checkbox-' + encodeURIComponent(file.filename));
+        fileReviewedLabel.appendChild(document.createTextNode(' Reviewed'));
+        fileReviewedLabel.appendTo(fileReviewed);
+    }
 
     if (file.extra) {
         var extraContainer = new Element(document.createElement('div'));
@@ -2230,7 +2236,8 @@ Splinter.showOverview = function () {
     Dom.getElementsByClassName('file', 'div', '', function (node) { 
         Dom.setStyle(node, 'display', 'none');
     });
-    Splinter.updateMyPatchComments();
+    if (!Splinter.readOnly)
+        Splinter.updateMyPatchComments();
 };
 
 Splinter.showAllFiles = function () {
@@ -2437,27 +2444,31 @@ Splinter.start = function () {
         Dom.setStyle('emptyCommentNotice', 'display', 'none');
     }
 
-    var myComment = Dom.get('myComment');
-    myComment.value = Splinter.theReview.intro ? Splinter.theReview.intro : "";
-    Event.addListener(myComment, 'focus', function () {
-        Dom.setStyle('emptyCommentNotice', 'display', 'none');
-    });
-    Event.addListener(myComment, 'blur', function () {
-        if (myComment.value == '') {
-            Dom.setStyle('emptyCommentNotice', 'display', 'block');
-        }
-    });
-    Event.addListener(myComment, 'keydown', function () {
-        Splinter.queueSaveDraft();
-        Splinter.queueUpdateHaveDraft();
-    });
+    if (!Splinter.readOnly) {
+        var myComment = Dom.get('myComment');
+        myComment.value = Splinter.theReview.intro ? Splinter.theReview.intro : "";
+        Event.addListener(myComment, 'focus', function () {
+            Dom.setStyle('emptyCommentNotice', 'display', 'none');
+        });
+        Event.addListener(myComment, 'blur', function () {
+            if (myComment.value == '') {
+                Dom.setStyle('emptyCommentNotice', 'display', 'block');
+            }
+        });
+        Event.addListener(myComment, 'keydown', function () {
+            Splinter.queueSaveDraft();
+            Splinter.queueUpdateHaveDraft();
+        });
 
-    Splinter.updateMyPatchComments();
+        Splinter.updateMyPatchComments();
 
-    Splinter.queueUpdateHaveDraft();
+        Splinter.queueUpdateHaveDraft();
 
-    Event.addListener("publishButton", "click", Splinter.publishReview);
-    Event.addListener("cancelButton", "click", Splinter.discardReview);
+        Event.addListener("publishButton", "click", Splinter.publishReview);
+        Event.addListener("cancelButton", "click", Splinter.discardReview);
+    } else {
+        Dom.setStyle('haveDraftNotice', 'display', 'none');
+    }
 };
 
 Splinter.newPageUrl = function (newBugId, newAttachmentId) {