From 778afb699f08d6b8e7f1ac5e77e927f45e11f8be Mon Sep 17 00:00:00 2001
From: Ciro Santilli <ciro.santilli@gmail.com>
Date: Fri, 19 Sep 2014 15:26:20 +0200
Subject: [PATCH] Replace javascript:; links with buttons.

---
 app/assets/stylesheets/sections/notes.scss    |  1 +
 app/helpers/notes_helper.rb                   |  8 ++++----
 features/steps/project/merge_requests.rb      |  5 +----
 features/steps/shared/diff_note.rb            |  4 ++--
 spec/features/notes_on_merge_requests_spec.rb | 15 ++++++++++-----
 5 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss
index 8df25f53762..50a24c2c4d8 100644
--- a/app/assets/stylesheets/sections/notes.scss
+++ b/app/assets/stylesheets/sections/notes.scss
@@ -145,6 +145,7 @@ ul.notes {
 .diff-file tr.line_holder {
   .add-diff-note {
     background: image-url("diff_note_add.png") no-repeat left 0;
+    border: none;
     height: 22px;
     margin-left: -65px;
     position: absolute;
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index cddcae464b0..15d4b875c4c 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -52,8 +52,8 @@ module NotesHelper
       discussion_id: discussion_id
     }
 
-    link_to "", "javascript:;", class: "add-diff-note js-add-diff-note-button",
-      data: data, title: "Add a comment to this line"
+    button_tag '', class: 'btn add-diff-note js-add-diff-note-button',
+               data: data, title: 'Add a comment to this line'
   end
 
   def link_to_reply_diff(note)
@@ -67,8 +67,8 @@ module NotesHelper
       discussion_id: note.discussion_id
     }
 
-    link_to "javascript:;", class: "btn reply-btn js-discussion-reply-button",
-      data: data, title: "Add a reply" do
+    button_tag class: 'btn reply-btn js-discussion-reply-button',
+               data: data, title: 'Add a reply' do
       link_text = content_tag(:i, nil, class: 'icon-comment')
       link_text << ' Reply'
     end
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index da3e5ec8037..cd8475c14ac 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -4,6 +4,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   include SharedNote
   include SharedPaths
   include SharedMarkdown
+  include SharedDiffNote
 
   step 'I click link "New Merge Request"' do
     click_link "New Merge Request"
@@ -292,8 +293,4 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   def have_visible_content (text)
     have_css("*", text: text, visible: true)
   end
-
-  def click_diff_line(code)
-    find("a[data-line-code='#{code}']").click
-  end
 end
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
index 7d017669b7c..10f3ed90b56 100644
--- a/features/steps/shared/diff_note.rb
+++ b/features/steps/shared/diff_note.rb
@@ -103,7 +103,7 @@ module SharedDiffNote
 
   step 'I should see a discussion reply button' do
     within(diff_file_selector) do
-      page.should have_link("Reply")
+      page.should have_button('Reply')
     end
   end
 
@@ -160,6 +160,6 @@ module SharedDiffNote
   end
 
   def click_diff_line(code)
-    find("a[data-line-code='#{code}']").click
+    find("button[data-line-code='#{code}']").click
   end
 end
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index 47776ba7f3f..92f3a6c0929 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -133,7 +133,7 @@ describe 'Comments' do
 
     describe "when adding a note" do
       before do
-        find("a[data-line-code=\"#{line_code}\"]").click
+        click_diff_line
       end
 
       describe "the notes holder" do
@@ -144,7 +144,7 @@ describe 'Comments' do
 
       describe "the note form" do
         it "shouldn't add a second form for same row" do
-          find("a[data-line-code=\"#{line_code}\"]").click
+          click_diff_line
 
           should have_css("tr[id='#{line_code}'] + .js-temp-notes-holder form", count: 1)
         end
@@ -161,8 +161,8 @@ describe 'Comments' do
 
     describe "with muliple note forms" do
       before do
-        find("a[data-line-code=\"#{line_code}\"]").click
-        find("a[data-line-code=\"#{line_code_2}\"]").click
+        click_diff_line
+        click_diff_line(line_code_2)
       end
 
       it { should have_css(".js-temp-notes-holder", count: 2) }
@@ -193,7 +193,7 @@ describe 'Comments' do
           should have_content("Another comment on line 10")
           should have_css(".notes_holder")
           should have_css(".notes_holder .note", count: 1)
-          should have_link("Reply")
+          should have_button('Reply')
         end
       end
     end
@@ -206,4 +206,9 @@ describe 'Comments' do
   def line_code_2
     sample_compare.changes.last[:line_code]
   end
+
+  def click_diff_line(data = nil)
+    data ||= line_code
+    find("button[data-line-code=\"#{data}\"]").click
+  end
 end