Patchwork D3437: paper: don't register click handlers with inline javascript (issue5812)

login
register
mail settings
Submitter phabricator
Date May 3, 2018, 2:16 a.m.
Message ID <differential-rev-PHID-DREV-kzgbcwh2cw3gxyhq33aa-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/31255/
State Superseded
Headers show

Comments

phabricator - May 3, 2018, 2:16 a.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The use of inline href="javascript:" undermines CSP policies that
  don't allow inline javascript.
  
  This commit changes the registering of the diffstat and line wrapping
  toggle handlers to the the global DOMContentLoaded handler, thus
  eliminating all inline javascript from the paper template.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3437

AFFECTED FILES
  mercurial/templates/paper/changeset.tmpl
  mercurial/templates/paper/filediff.tmpl
  mercurial/templates/paper/filerevision.tmpl
  mercurial/templates/static/mercurial.js
  tests/test-hgweb-commands.t
  tests/test-hgweb-diffs.t
  tests/test-hgweb-removed.t

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - May 3, 2018, 2:21 a.m.
krbullock accepted this revision.
krbullock added a comment.
This revision is now accepted and ready to land.


  Queued, thanks.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3437

To: indygreg, #hg-reviewers, krbullock
Cc: krbullock, mercurial-devel
phabricator - May 3, 2018, 3:45 a.m.
yuja added a comment.


  Maybe needs `href="#"` to make it look like a link?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3437

To: indygreg, #hg-reviewers, krbullock
Cc: yuja, krbullock, mercurial-devel
phabricator - May 3, 2018, 4:01 a.m.
indygreg added a comment.


  In https://phab.mercurial-scm.org/D3437#54736, @yuja wrote:
  
  > Maybe needs `href="#"` to make it look like a link?
  
  
  Good catch. I submitted a follow-up at https://phab.mercurial-scm.org/D3438. Feel free to roll into this one on hg-committed if it looks good to you.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3437

To: indygreg, #hg-reviewers, krbullock
Cc: yuja, krbullock, mercurial-devel

Patch

diff --git a/tests/test-hgweb-removed.t b/tests/test-hgweb-removed.t
--- a/tests/test-hgweb-removed.t
+++ b/tests/test-hgweb-removed.t
@@ -103,9 +103,9 @@ 
     <td class="diffstat">
        1 files changed, 0 insertions(+), 1 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
+      <a id="diffstatexpand" class="diffstattoggle">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <a class="diffstattoggle">[<tt>-</tt>]</a>
         <table class="diffstat-table stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">a</a></td>
       <td class="diffstat-total" align="right">1</td>
@@ -121,7 +121,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line diff</div>
   <div class="stripes2 diffblocks">
   <div class="bottomline inc-lineno"><pre class="sourcelines wrap">
@@ -225,7 +225,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line diff</div>
   <div class="stripes2 diffblocks">
   <div class="bottomline inc-lineno"><pre class="sourcelines wrap">
diff --git a/tests/test-hgweb-diffs.t b/tests/test-hgweb-diffs.t
--- a/tests/test-hgweb-diffs.t
+++ b/tests/test-hgweb-diffs.t
@@ -122,9 +122,9 @@ 
     <td class="diffstat">
        2 files changed, 2 insertions(+), 0 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
+      <a id="diffstatexpand" class="diffstattoggle">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <a class="diffstattoggle">[<tt>-</tt>]</a>
         <table class="diffstat-table stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">a</a></td>
       <td class="diffstat-total" align="right">1</td>
@@ -148,7 +148,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line diff</div>
   <div class="stripes2 diffblocks">
   <div class="bottomline inc-lineno"><pre class="sourcelines wrap">
@@ -289,7 +289,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line diff</div>
   <div class="stripes2 diffblocks">
   <div class="bottomline inc-lineno"><pre class="sourcelines wrap">
@@ -419,9 +419,9 @@ 
     <td class="diffstat">
        2 files changed, 2 insertions(+), 0 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
+      <a id="diffstatexpand" class="diffstattoggle">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <a class="diffstattoggle">[<tt>-</tt>]</a>
         <table class="diffstat-table stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">a</a></td>
       <td class="diffstat-total" align="right">1</td>
@@ -445,7 +445,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line diff</div>
   <div class="stripes2 diffblocks">
   <div class="bottomline inc-lineno"><pre class="sourcelines wrap">
@@ -590,7 +590,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line diff</div>
   <div class="stripes2 diffblocks">
   <div class="bottomline inc-lineno"><pre class="sourcelines wrap">
diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t
--- a/tests/test-hgweb-commands.t
+++ b/tests/test-hgweb-commands.t
@@ -916,9 +916,9 @@ 
     <td class="diffstat">
        2 files changed, 2 insertions(+), 0 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
+      <a id="diffstatexpand" class="diffstattoggle">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <a class="diffstattoggle">[<tt>-</tt>]</a>
         <table class="diffstat-table stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">da/foo</a></td>
       <td class="diffstat-total" align="right">1</td>
@@ -942,7 +942,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line diff</div>
   <div class="stripes2 diffblocks">
   <div class="bottomline inc-lineno"><pre class="sourcelines wrap">
@@ -1342,7 +1342,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line source</div>
   <pre class="sourcelines stripes4 wrap bottomline"
        data-logurl="/log/1/foo"
@@ -1476,7 +1476,7 @@ 
   </table>
   
   <div class="overflow">
-  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+  <div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
   <div class="sourcefirst"> line source</div>
   <pre class="sourcelines stripes4 wrap bottomline"
        data-logurl="/log/2/foo"
diff --git a/mercurial/templates/static/mercurial.js b/mercurial/templates/static/mercurial.js
--- a/mercurial/templates/static/mercurial.js
+++ b/mercurial/templates/static/mercurial.js
@@ -551,6 +551,28 @@ 
     form.style.display = 'block';
 }
 
+function addDiffStatToggle() {
+    var els = document.getElementsByClassName("diffstattoggle");
+
+    for (var i = 0; i < els.length; i++) {
+        els[i].addEventListener("click", toggleDiffstat, false);
+    }
+}
+
+function addLineWrapToggle() {
+    var els = document.getElementsByClassName("linewraptoggle");
+
+    for (var i = 0; i < els.length; i++) {
+        var nodes = els[i].getElementsByClassName("linewraplink");
+
+        for (var j = 0; j < nodes.length; j++) {
+            nodes[j].addEventListener("click", toggleLinewrap, false);
+        }
+    }
+}
+
 document.addEventListener('DOMContentLoaded', function() {
    process_dates();
+   addDiffStatToggle();
+   addLineWrapToggle();
 }, false);
diff --git a/mercurial/templates/paper/filerevision.tmpl b/mercurial/templates/paper/filerevision.tmpl
--- a/mercurial/templates/paper/filerevision.tmpl
+++ b/mercurial/templates/paper/filerevision.tmpl
@@ -65,7 +65,7 @@ 
 </table>
 
 <div class="overflow">
-<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
 <div class="sourcefirst"> line source</div>
 <pre class="sourcelines stripes4 wrap bottomline"
      data-logurl="{url|urlescape}log/{symrev}/{file|urlescape}"
diff --git a/mercurial/templates/paper/filediff.tmpl b/mercurial/templates/paper/filediff.tmpl
--- a/mercurial/templates/paper/filediff.tmpl
+++ b/mercurial/templates/paper/filediff.tmpl
@@ -65,7 +65,7 @@ 
 </table>
 
 <div class="overflow">
-<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
 <div class="sourcefirst"> line diff</div>
 <div class="stripes2 diffblocks">
 {diff}
diff --git a/mercurial/templates/paper/changeset.tmpl b/mercurial/templates/paper/changeset.tmpl
--- a/mercurial/templates/paper/changeset.tmpl
+++ b/mercurial/templates/paper/changeset.tmpl
@@ -73,17 +73,17 @@ 
   <th class="diffstat">diffstat</th>
   <td class="diffstat">
     {diffsummary}
-    <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
+    <a id="diffstatexpand" class="diffstattoggle">[<tt>+</tt>]</a>
     <div id="diffstatdetails" style="display:none;">
-      <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+      <a class="diffstattoggle">[<tt>-</tt>]</a>
       <table class="diffstat-table stripes2">{diffstat}</table>
     </div>
   </td>
 </tr>
 </table>
 
 <div class="overflow">
-<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink" href="javascript:toggleLinewrap()">on</a></div>
+<div class="sourcefirst linewraptoggle">line wrap: <a class="linewraplink">on</a></div>
 <div class="sourcefirst"> line diff</div>
 <div class="stripes2 diffblocks">
 {diff}