Patchwork [3,of,3] hgweb: correct markup in templates

login
register
mail settings
Submitter Anton Shestakov
Date Jan. 9, 2015, 5:20 p.m.
Message ID <54d5c8d928e0604dbe06.1420824028@neuro>
Download mbox | patch
Permalink /patch/7406/
State Changes Requested
Headers show

Comments

Anton Shestakov - Jan. 9, 2015, 5:20 p.m.
# HG changeset patch
# User Anton Shestakov <engored@ya.ru>
# Date 1420804725 -28800
#      Fri Jan 09 19:58:45 2015 +0800
# Node ID 54d5c8d928e0604dbe0611ae05ff9327aa6902ab
# Parent  804c0b12828662447f52973f1ea11ee42b9534b6
hgweb: correct markup in templates

Templates declare xhtml doctype, which means, in particular, that the document
must also be valid xml. So <img> elements must be closed.

On the other hand, '<a .../>foo</a>' is incorrect, since the first tag just
tries to close itself. It doesn't work, either because web browsers know better
than this or because there should be a whitespace before /: '<a />'. So for the
web interface user the links looked normal, but now they do in code as well.

<p> elements can only contain inline elements, so as soon as browser encounters
a block element (e.g. block <div>) "inside" a <p>, it puts an implicit </p>.
It's better to do this explicitly.

In spartan/filelogentry.tmpl <th> element was mistakingly closed with </td>.
Matt Mackall - Jan. 9, 2015, 9:20 p.m.
On Sat, 2015-01-10 at 01:20 +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <engored@ya.ru>
> # Date 1420804725 -28800
> #      Fri Jan 09 19:58:45 2015 +0800
> # Node ID 54d5c8d928e0604dbe0611ae05ff9327aa6902ab
> # Parent  804c0b12828662447f52973f1ea11ee42b9534b6
> hgweb: correct markup in templates
> 
> Templates declare xhtml doctype, which means, in particular, that the document
> must also be valid xml. So <img> elements must be closed.
> 
> On the other hand, '<a .../>foo</a>' is incorrect, since the first tag just
> tries to close itself. It doesn't work, either because web browsers know better
> than this or because there should be a whitespace before /: '<a />'. So for the
> web interface user the links looked normal, but now they do in code as well.
> 
> <p> elements can only contain inline elements, so as soon as browser encounters
> a block element (e.g. block <div>) "inside" a <p>, it puts an implicit </p>.
> It's better to do this explicitly.
> 
> In spartan/filelogentry.tmpl <th> element was mistakingly closed with </td>.

This sounds great, but I'm afraid we need this to be four patches for
review purposes. One per paragraph above, please.

Patch

diff --git a/mercurial/templates/monoblue/footer.tmpl b/mercurial/templates/monoblue/footer.tmpl
--- a/mercurial/templates/monoblue/footer.tmpl
+++ b/mercurial/templates/monoblue/footer.tmpl
@@ -9,7 +9,7 @@ 
     </div>
 
     <div id="powered-by">
-        <p><a href="{logourl}" title="Mercurial"><img src="{staticurl|urlescape}{logoimg}" width=75 height=90 border=0 alt="mercurial"></a></p>
+        <p><a href="{logourl}" title="Mercurial"><img src="{staticurl|urlescape}{logoimg}" width=75 height=90 border=0 alt="mercurial" /></a></p>
     </div>
 
     <div id="corner-top-left"></div>
diff --git a/mercurial/templates/paper/bookmarks.tmpl b/mercurial/templates/paper/bookmarks.tmpl
--- a/mercurial/templates/paper/bookmarks.tmpl
+++ b/mercurial/templates/paper/bookmarks.tmpl
@@ -23,10 +23,10 @@ 
 <ul>
 <li><a href="{url|urlescape}help{sessionvars%urlparameter}">help</a></li>
 </ul>
-<p>
+<p></p>
 <div class="atom-logo">
 <a href="{url|urlescape}atom-bookmarks" title="subscribe to atom feed">
-<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed">
+<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed" />
 </a>
 </div>
 </div>
diff --git a/mercurial/templates/paper/branches.tmpl b/mercurial/templates/paper/branches.tmpl
--- a/mercurial/templates/paper/branches.tmpl
+++ b/mercurial/templates/paper/branches.tmpl
@@ -23,10 +23,10 @@ 
 <ul>
  <li><a href="{url|urlescape}help{sessionvars%urlparameter}">help</a></li>
 </ul>
-<p>
+<p></p>
 <div class="atom-logo">
 <a href="{url|urlescape}atom-branches" title="subscribe to atom feed">
-<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed">
+<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed" />
 </a>
 </div>
 </div>
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
@@ -65,10 +65,10 @@ 
   <th class="diffstat">diffstat</th>
   <td class="diffstat">
     {diffsummary}
-    <a id="diffstatexpand" href="javascript:toggleDiffstat()"/>[<tt>+</tt>]</a>
+    <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
     <div id="diffstatdetails" style="display:none;">
-      <a href="javascript:toggleDiffstat()"/>[<tt>-</tt>]</a>
-      <p>
+      <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+      <p></p>
       <table class="stripes2">{diffstat}</table>
     </div>
   </td>
diff --git a/mercurial/templates/paper/filelog.tmpl b/mercurial/templates/paper/filelog.tmpl
--- a/mercurial/templates/paper/filelog.tmpl
+++ b/mercurial/templates/paper/filelog.tmpl
@@ -35,10 +35,11 @@ 
 <ul>
 <li><a href="{url|urlescape}help{sessionvars%urlparameter}">help</a></li>
 </ul>
-<p>
+<p></p>
 <div class="atom-logo">
 <a href="{url|urlescape}atom-log/{node|short}/{file|urlescape}" title="subscribe to atom feed">
-<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed"></a>
+<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed" />
+</a>
 </div>
 </div>
 
diff --git a/mercurial/templates/paper/graph.tmpl b/mercurial/templates/paper/graph.tmpl
--- a/mercurial/templates/paper/graph.tmpl
+++ b/mercurial/templates/paper/graph.tmpl
@@ -28,10 +28,10 @@ 
 <ul>
  <li><a href="{url|urlescape}help{sessionvars%urlparameter}">help</a></li>
 </ul>
-<p>
+<p></p>
 <div class="atom-logo">
 <a href="{url|urlescape}atom-log" title="subscribe to atom feed">
-<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed">
+<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed" />
 </a>
 </div>
 </div>
diff --git a/mercurial/templates/paper/shortlog.tmpl b/mercurial/templates/paper/shortlog.tmpl
--- a/mercurial/templates/paper/shortlog.tmpl
+++ b/mercurial/templates/paper/shortlog.tmpl
@@ -30,10 +30,10 @@ 
 <ul>
  <li><a href="{url|urlescape}help{sessionvars%urlparameter}">help</a></li>
 </ul>
-<p>
+<p></p>
 <div class="atom-logo">
 <a href="{url|urlescape}atom-log" title="subscribe to atom feed">
-<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed">
+<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed" />
 </a>
 </div>
 </div>
diff --git a/mercurial/templates/paper/tags.tmpl b/mercurial/templates/paper/tags.tmpl
--- a/mercurial/templates/paper/tags.tmpl
+++ b/mercurial/templates/paper/tags.tmpl
@@ -23,10 +23,11 @@ 
 <ul>
 <li><a href="{url|urlescape}help{sessionvars%urlparameter}">help</a></li>
 </ul>
-<p>
+<p></p>
 <div class="atom-logo">
 <a href="{url|urlescape}atom-tags" title="subscribe to atom feed">
-<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed"></a>
+<img class="atom-logo" src="{staticurl|urlescape}feed-icon-14x14.png" alt="atom feed" />
+</a>
 </div>
 </div>
 
diff --git a/mercurial/templates/spartan/filelogentry.tmpl b/mercurial/templates/spartan/filelogentry.tmpl
--- a/mercurial/templates/spartan/filelogentry.tmpl
+++ b/mercurial/templates/spartan/filelogentry.tmpl
@@ -4,7 +4,7 @@ 
   <th class="firstline"><a href="{url|urlescape}rev/{node|short}{sessionvars%urlparameter}">{desc|strip|firstline|escape|nonempty}</a></th>
  </tr>
  <tr>
-  <th class="revision">revision {filerev}:</td>
+  <th class="revision">revision {filerev}:</th>
   <td class="node">
    <a href="{url|urlescape}file/{node|short}/{file|urlescape}{sessionvars%urlparameter}">{node|short}</a>
    <a href="{url|urlescape}diff/{node|short}/{file|urlescape}{sessionvars%urlparameter}">(diff)</a>
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
@@ -726,10 +726,10 @@  Logs and changes
   <ul>
    <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed">
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
   </a>
   </div>
   </div>
@@ -891,10 +891,10 @@  Logs and changes
     <td class="diffstat">
        2 files changed, 2 insertions(+), 0 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()"/>[<tt>+</tt>]</a>
+      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()"/>[<tt>-</tt>]</a>
-        <p>
+        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <p></p>
         <table class="stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">da/foo</a></td>
       <td class="diffstat-total" align="right">1</td>
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
@@ -115,10 +115,10 @@  revision
     <td class="diffstat">
        2 files changed, 2 insertions(+), 0 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()"/>[<tt>+</tt>]</a>
+      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()"/>[<tt>-</tt>]</a>
-        <p>
+        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <p></p>
         <table class="stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">a</a></td>
       <td class="diffstat-total" align="right">1</td>
@@ -387,10 +387,10 @@  revision
     <td class="diffstat">
        2 files changed, 2 insertions(+), 0 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()"/>[<tt>+</tt>]</a>
+      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()"/>[<tt>-</tt>]</a>
-        <p>
+        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <p></p>
         <table class="stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">a</a></td>
       <td class="diffstat-total" align="right">1</td>
diff --git a/tests/test-hgweb-empty.t b/tests/test-hgweb-empty.t
--- a/tests/test-hgweb-empty.t
+++ b/tests/test-hgweb-empty.t
@@ -48,10 +48,10 @@  Some tests for hgweb in an empty reposit
   <ul>
    <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed">
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
   </a>
   </div>
   </div>
@@ -158,10 +158,10 @@  Some tests for hgweb in an empty reposit
   <ul>
    <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed">
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
   </a>
   </div>
   </div>
@@ -264,10 +264,10 @@  Some tests for hgweb in an empty reposit
   <ul>
    <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed">
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
   </a>
   </div>
   </div>
diff --git a/tests/test-hgweb-filelog.t b/tests/test-hgweb-filelog.t
--- a/tests/test-hgweb-filelog.t
+++ b/tests/test-hgweb-filelog.t
@@ -156,10 +156,11 @@  tip - two revisions
   <ul>
   <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log/01de2d66a28d/a" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed"></a>
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
+  </a>
   </div>
   </div>
   
@@ -265,10 +266,11 @@  second version - two revisions
   <ul>
   <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log/01de2d66a28d/a" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed"></a>
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
+  </a>
   </div>
   </div>
   
@@ -374,10 +376,11 @@  first deleted - one revision
   <ul>
   <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log/5ed941583260/a" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed"></a>
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
+  </a>
   </div>
   </div>
   
@@ -478,10 +481,11 @@  first version - one revision
   <ul>
   <li><a href="/help">help</a></li>
   </ul>
-  <p>
+  <p></p>
   <div class="atom-logo">
   <a href="/atom-log/5ed941583260/a" title="subscribe to atom feed">
-  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed"></a>
+  <img class="atom-logo" src="/static/feed-icon-14x14.png" alt="atom feed" />
+  </a>
   </div>
   </div>
   
@@ -643,7 +647,7 @@  should show base link, use spartan becau
     <th class="firstline"><a href="/rev/b7682196df1c?style=spartan">change c</a></th>
    </tr>
    <tr>
-    <th class="revision">revision 1:</td>
+    <th class="revision">revision 1:</th>
     <td class="node">
      <a href="/file/b7682196df1c/c?style=spartan">b7682196df1c</a>
      <a href="/diff/b7682196df1c/c?style=spartan">(diff)</a>
@@ -668,7 +672,7 @@  should show base link, use spartan becau
     <th class="firstline"><a href="/rev/1a6696706df2?style=spartan">mv b</a></th>
    </tr>
    <tr>
-    <th class="revision">revision 0:</td>
+    <th class="revision">revision 0:</th>
     <td class="node">
      <a href="/file/1a6696706df2/c?style=spartan">1a6696706df2</a>
      <a href="/diff/1a6696706df2/c?style=spartan">(diff)</a>
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
@@ -96,10 +96,10 @@  revision
     <td class="diffstat">
        1 files changed, 0 insertions(+), 1 deletions(-)
   
-      <a id="diffstatexpand" href="javascript:toggleDiffstat()"/>[<tt>+</tt>]</a>
+      <a id="diffstatexpand" href="javascript:toggleDiffstat()">[<tt>+</tt>]</a>
       <div id="diffstatdetails" style="display:none;">
-        <a href="javascript:toggleDiffstat()"/>[<tt>-</tt>]</a>
-        <p>
+        <a href="javascript:toggleDiffstat()">[<tt>-</tt>]</a>
+        <p></p>
         <table class="stripes2">  <tr>
       <td class="diffstat-file"><a href="#l1.1">a</a></td>
       <td class="diffstat-total" align="right">1</td>