Merge pull request #7793 from cirosantilli/replace-javascript

Replace javascript:; links with buttons.

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;

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

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

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

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