Merge pull request #41874 from jacobherrington/error-page-css-and-a11y-improvements

Error page CSS and a11y improvements

actionpack/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Remove inline styles and address basic accessibilty issues on rescue templates.
+
+    *Jacob Herrington*
+
 *   Add support for 'private, no-store' Cache-Control headers.
 
     Previously, 'no-store' was exclusive; no other directives could be specified.

actionpack/lib/action_dispatch/middleware/templates/rescues/_message_and_suggestions.html.erb

@@ -11,7 +11,7 @@
     <b>Did you mean?</b>
     <ul>
     <% corrections.each do |correction| %>
-      <li style="list-style-type: none"><%= h correction %></li>
+      <li class="correction"><%= h correction %></li>
     <% end %>
     </ul>
   <% end %>

actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb

@@ -1,17 +1,17 @@
-<h2 style="margin-top: 30px">Request</h2>
+<h2 class="request-heading">Request</h2>
 <% if params_valid? %>
   <p><b>Parameters</b>:</p> <pre><%= debug_params(@request.filtered_parameters) %></pre>
 <% end %>
 
 <div class="details">
   <div class="summary"><a href="#" onclick="return toggleSessionDump()">Toggle session dump</a></div>
-  <div id="session_dump" style="display:none"><pre><%= debug_hash @request.session %></pre></div>
+  <div id="session_dump" class="hidden"><pre><%= debug_hash @request.session %></pre></div>
 </div>
 
 <div class="details">
   <div class="summary"><a href="#" onclick="return toggleEnvDump()">Toggle env dump</a></div>
-  <div id="env_dump" style="display:none"><pre><%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %></pre></div>
+  <div id="env_dump" class="hidden"><pre><%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %></pre></div>
 </div>
 
-<h2 style="margin-top: 30px">Response</h2>
+<h2 class="response-heading">Response</h2>
 <p><b>Headers</b>:</p> <pre><%= debug_headers(defined?(@response) ? @response.headers : {}) %></pre>

actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb

@@ -14,7 +14,7 @@
 
   <% traces.each do |name, trace| %>
     <div id="<%= "#{name.gsub(/\s/, '-')}-#{error_index}" %>" style="display: <%= (name == trace_to_show) ? 'block' : 'none' %>;">
-      <code style="font-size: 11px;">
+      <code class="traces">
         <% trace.each do |frame| %>
           <a class="trace-frames trace-frames-<%= error_index %>" data-exception-object-id="<%= frame[:exception_object_id] %>" data-frame-id="<%= frame[:id] %>" href="#">
             <%= frame[:trace] %>

actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb

@@ -1,7 +1,7 @@
 <header>
   <h1>Blocked host: <%= @host %></h1>
 </header>
-<div id="container">
+<main role="main" id="container">
   <h2>To allow requests to <%= @host %>, add the following to your environment configuration:</h2>
   <pre>config.hosts &lt;&lt; "<%= @host %>"</pre>
-</div>
+</main>

actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb

@@ -7,7 +7,7 @@
   </h1>
 </header>
 
-<div id="container">
+<main role="main" id="container">
   <%= render "rescues/message_and_suggestions", exception: @exception %>
   <%= render "rescues/actions", exception: @exception, request: @request %>
 
@@ -20,16 +20,16 @@
 
   <% @exception_wrapper.wrapped_causes.each.with_index(1) do |wrapper, index| %>
     <div class="details">
-      <a class="summary" href="#" style="color: #F0F0F0; text-decoration: none; background: #C52F24; border-bottom: none;" onclick="return toggle(<%= wrapper.exception.object_id %>)">
+      <a class="summary" href="#" onclick="return toggle(<%= wrapper.exception.object_id %>)">
         <%= wrapper.exception.class.name %>: <%= h wrapper.exception.message %>
       </a>
     </div>
 
-    <div id="<%= wrapper.exception.object_id %>" style="display: none;">
+    <div id="<%= wrapper.exception.object_id %>" class="hidden">
       <%= render "rescues/source", source_extracts: wrapper.source_extracts, show_source_idx: wrapper.source_to_show_id, error_index: index %>
       <%= render "rescues/trace", traces: wrapper.traces, trace_to_show: wrapper.trace_to_show, error_index: index %>
     </div>
   <% end %>
 
   <%= render template: "rescues/_request_and_response" %>
-</div>
+</main>

actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb

@@ -1,4 +1,4 @@
-<header>
+<header role="banner">
   <h1>
     <%= @exception.class.to_s %>
     <% if @request.parameters['controller'] %>
@@ -7,7 +7,7 @@
   </h1>
 </header>
 
-<div id="container">
+<main role="main" id="container">
   <h2>
     <%= h @exception.message %>
     <% if defined?(ActiveStorage) && @exception.message.match?(%r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}}) %>
@@ -21,4 +21,4 @@
   <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
   <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
-</div>
+</main>

actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb

@@ -49,6 +49,14 @@
       line-height: 25px;
     }
 
+    code.traces {
+      font-size: 11px;
+    }
+
+    .response-heading, .request-heading {
+      margin-top: 30px;
+    }
+
     .exception-message {
       padding: 8px 0;
     }
@@ -75,6 +83,13 @@
       display: block;
     }
 
+    a.summary {
+      color: #F0F0F0;
+      text-decoration: none;
+      background: #C52F24;
+      border-bottom: none;
+    }
+
     .details pre {
       margin: 5px;
       border: none;
@@ -114,7 +129,7 @@
 
     .source .data .line_numbers {
       background-color: #ECECEC;
-      color: #AAA;
+      color: #555;
       padding: 1em .5em;
       border-right: 1px solid #DDD;
       text-align: right;
@@ -143,6 +158,10 @@
       display: none;
     }
 
+    .correction {
+      list-style-type: none;
+    }
+
     input[type="submit"] {
       color: white;
       background-color: #C00;
@@ -165,15 +184,14 @@
       transform: translateY(1px)
     }
 
-
     a { color: #980905; }
     a:visited { color: #666; }
     a.trace-frames {
       color: #666;
       overflow-wrap: break-word;
     }
-    a:hover { color: #C00; }
-    a.trace-frames.selected { color: #C00 }
+    a:hover, a.trace-frames.selected { color: #C00; }
+    a.summary:hover { color: #FFF; }
 
     @media (prefers-color-scheme: dark) {
       body {
@@ -181,11 +199,7 @@
         color: #ECECEC;
       }
 
-      .details {
-        border-color: #666;
-      }
-
-      .summary {
+      .details, .summary {
         border-color: #666;
       }
 
@@ -220,8 +234,7 @@
 
       a { color: #C00; }
       a.trace-frames { color: #999; }
-      a:hover { color: #E9382B; }
-      a.trace-frames.selected { color: #E9382B; }
+      a:hover, a.trace-frames.selected { color: #E9382B; }
     }
 
     <%= yield :style %>
@@ -229,8 +242,7 @@
 
   <script>
     var toggle = function(id) {
-      var s = document.getElementById(id).style;
-      s.display = s.display == 'none' ? 'block' : 'none';
+      document.getElementById(id).classList.toggle('hidden');
       return false;
     }
     var show = function(id) {

actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.html.erb

@@ -1,8 +1,8 @@
-<header>
+<header role="banner">
   <h1>No template for interactive request</h1>
 </header>
 
-<div id="container">
+<main id="container">
   <h2><%= h @exception.message %></h2>
 
   <p class="summary">
@@ -16,4 +16,4 @@
     since we expect an HTML template
     to be rendered for such requests. If that's the case, carry on.
   </p>
-</div>
+</main>

actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb

@@ -1,11 +1,11 @@
-<header>
+<header role="banner">
   <h1>Template is missing</h1>
 </header>
 
-<div id="container">
+<main role="main" id="container">
   <h2><%= h @exception.message %></h2>
 
   <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
   <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
-</div>
+</main>

actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb

@@ -1,7 +1,7 @@
-<header>
+<header role="banner">
   <h1>Routing Error</h1>
 </header>
-<div id="container">
+<main role="main" id="container">
   <h2><%= h @exception.message %></h2>
   <% unless @exception.failures.empty? %>
     <p>
@@ -29,4 +29,4 @@
   <% end %>
 
   <%= render template: "rescues/_request_and_response" %>
-</div>
+</main>

actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb

@@ -1,11 +1,11 @@
-<header>
+<header role="banner">
   <h1>
     <%= @exception.cause.class.to_s %> in
     <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %>
   </h1>
 </header>
 
-<div id="container">
+<main role="main" id="container">
   <p>
     Showing <i><%= @exception.file_name %></i> where line <b>#<%= @exception.line_number %></b> raised:
   </p>
@@ -17,4 +17,4 @@
 
   <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
-</div>
+</main>

actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.html.erb

@@ -1,6 +1,6 @@
-<header>
+<header role="banner">
   <h1>Unknown action</h1>
 </header>
-<div id="container">
+<main role="main" id="container">
   <%= render "rescues/message_and_suggestions", exception: @exception %>
-</div>
+</main>