Merge pull request #440 from lastobelus/add-target-to-editor-link

Allow editor links to work inside an iframe or with CSP that prohibits other protocols

README.md

@@ -35,6 +35,28 @@ end
 
 _Note: If you discover that Better Errors isn't working - particularly after upgrading from version 0.5.0 or less - be sure to set `config.consider_all_requests_local = true` in `config/environments/development.rb`._
 
+### Optional: Set `EDITOR`
+
+For many reasons outside of Better Errors, you should have the `EDITOR` environment variable set to your preferred
+editor.
+Better Errors, like many other tools, will use that environment variable to show a link that opens your
+editor to the file and line from the console.
+
+By default the links will open TextMate-protocol links.
+
+To see if your editor is supported or to set up a different editor, see [the wiki](https://github.com/BetterErrors/better_errors/wiki/Link-to-your-editor).
+
+### Optional: Set `BETTER_ERRORS_INSIDE_FRAME`
+
+If your application is running inside of an iframe, or if you have a Content Security Policy that disallows links
+to other protocols, the editor links will not work.
+
+To work around this set `BETTER_ERRORS_INSIDE_FRAME=1` in the environment, and the links will include `target=_blank`,
+allowing the link to open regardless of the policy.
+
+_This works because it opens the editor from a new browser tab, escaping from the restrictions of your site._
+_Unfortunately it leaves behind an empty tab each time, so only use this if needed._
+
 ## Security
 
 **NOTE:** It is *critical* you put better\_errors only in the **development** section of your Gemfile.

lib/better_errors/templates/variable_info.erb

@@ -1,7 +1,14 @@
 <header class="trace_info clearfix">
     <div class="title">
         <h2 class="name"><%= @frame.name %></h2>
-        <div class="location"><span class="filename"><a href="<%= editor_url(@frame) %>"><%= @frame.pretty_path %></a></span></div>
+        <div class="location">
+          <span class="filename">
+            <a
+              href="<%= editor_url(@frame) %>"
+              <%= ENV.key?('BETTER_ERRORS_INSIDE_FRAME') ? "target=_blank" : '' %>
+            ><%= @frame.pretty_path %></a>
+          </span>
+        </div>
     </div>
     <div class="code_block clearfix">
         <%== html_formatted_code_block @frame %>

spec/better_errors/error_page_spec.rb

@@ -76,15 +76,44 @@ module BetterErrors
     end
 
     context "variable inspection" do
+      let(:html) { error_page.do_variables("index" => 0)[:html] }
       let(:exception) { exception_binding.eval("raise") rescue $! }
 
+      it 'includes an editor link for the full path of the current frame' do
+        expect(html).to have_tag('.location .filename') do
+          with_tag('a[href*="better_errors"]')
+        end
+      end
+
+      context 'when BETTER_ERRORS_INSIDE_FRAME is set in the environment' do
+        before do
+          ENV['BETTER_ERRORS_INSIDE_FRAME'] = '1'
+        end
+        after do
+          ENV['BETTER_ERRORS_INSIDE_FRAME'] = nil
+        end
+
+        it 'includes an editor link with target=_blank' do
+          expect(html).to have_tag('.location .filename') do
+            with_tag('a[href*="better_errors"][target="_blank"]')
+          end
+        end
+      end
+
+      context 'when BETTER_ERRORS_INSIDE_FRAME is not set in the environment' do
+        it 'includes an editor link without target=_blank' do
+          expect(html).to have_tag('.location .filename') do
+            with_tag('a[href*="better_errors"]:not([target="_blank"])')
+          end
+        end
+      end
+
       context "when binding_of_caller is loaded" do
         before do
           skip "binding_of_caller is not loaded" unless BetterErrors.binding_of_caller_available?
         end
 
         it "shows local variables" do
-          html = error_page.do_variables("index" => 0)[:html]
           expect(html).to have_tag('div.variables tr') do
             with_tag('td.name', text: 'local_a')
             with_tag('pre', text: ':value_for_local_a')
@@ -96,7 +125,6 @@ module BetterErrors
         end
 
         it "shows instance variables" do
-          html = error_page.do_variables("index" => 0)[:html]
           expect(html).to have_tag('div.variables tr') do
             with_tag('td.name', text: '@inst_c')
             with_tag('pre', text: ':value_for_inst_c')
@@ -123,7 +151,6 @@ module BetterErrors
           }
 
           it "does not include that value" do
-            html = error_page.do_variables("index" => 0)[:html]
             expect(html).to have_tag('div.variables tr') do
               with_tag('td.name', text: 'local_a')
               with_tag('pre', text: ':value_for_local_a')
@@ -147,7 +174,6 @@ module BetterErrors
 
         it "does not show filtered variables" do
           allow(BetterErrors).to receive(:ignored_instance_variables).and_return([:@inst_d])
-          html = error_page.do_variables("index" => 0)[:html]
           expect(html).to have_tag('div.variables tr') do
             with_tag('td.name', text: '@inst_c')
             with_tag('pre', text: ':value_for_inst_c')
@@ -174,7 +200,6 @@ module BetterErrors
               let(:content) { 'A' * 480 }
 
               it "shows the variable content" do
-                html = error_page.do_variables("index" => 0)[:html]
                 expect(html).to have_tag('div.variables', text: %r{#{content}})
               end
             end
@@ -199,7 +224,6 @@ def inspect
                 }
 
                 it "shows the variable content" do
-                  html = error_page.do_variables("index" => 0)[:html]
                   expect(html).to have_tag('div.variables', text: /shortval/)
                 end
               end
@@ -212,7 +236,6 @@ def inspect
                 let(:content) { 'A' * 1101 }
 
                 it "includes an indication that the variable was too large" do
-                  html = error_page.do_variables("index" => 0)[:html]
                   expect(html).not_to have_tag('div.variables', text: %r{#{content}})
                   expect(html).to have_tag('div.variables', text: /Object too large/)
                 end
@@ -230,7 +253,6 @@ def initialize
                 }
 
                 it "does not attempt to show the class name" do
-                  html = error_page.do_variables("index" => 0)[:html]
                   expect(html).to have_tag('div.variables tr') do
                     with_tag('td.name', text: '@big_anonymous')
                     with_tag('.unsupported', text: /Object too large/)
@@ -258,7 +280,6 @@ def initialize
               let(:content) { 'A' * 480 }
 
               it "shows the variable content" do
-                html = error_page.do_variables("index" => 0)[:html]
                 expect(html).to have_tag('div.variables', text: %r{#{content}})
               end
             end
@@ -283,7 +304,6 @@ def inspect
                 }
 
                 it "shows the variable content" do
-                  html = error_page.do_variables("index" => 0)[:html]
                   expect(html).to have_tag('div.variables', text: /shortval/)
                 end
               end
@@ -297,7 +317,6 @@ def inspect
 
                 it "includes an indication that the variable was too large" do
 
-                  html = error_page.do_variables("index" => 0)[:html]
                   expect(html).not_to have_tag('div.variables', text: %r{#{content}})
                   expect(html).to have_tag('div.variables', text: /Object too large/)
                 end
@@ -316,7 +335,6 @@ def initialize
               }
 
               it "does not attempt to show the class name" do
-                html = error_page.do_variables("index" => 0)[:html]
                 expect(html).to have_tag('div.variables tr') do
                   with_tag('td.name', text: '@big_anonymous')
                   with_tag('.unsupported', text: /Object too large/)
@@ -340,7 +358,6 @@ def initialize
           let(:content) { 'A' * 100_001 }
 
           it "includes the content of large variables" do
-            html = error_page.do_variables("index" => 0)[:html]
             expect(html).to have_tag('div.variables', text: %r{#{content}})
             expect(html).not_to have_tag('div.variables', text: /Object too large/)
           end
@@ -353,7 +370,6 @@ def initialize
         end
 
         it "tells the user to add binding_of_caller to their gemfile to get fancy features" do
-          html = error_page.do_variables("index" => 0)[:html]
           expect(html).not_to have_tag('div.variables', text: /gem "binding_of_caller"/)
         end
       end