DeanHere / rietveld

Automatically exported from code.google.com/p/rietveld
Apache License 2.0
0 stars 0 forks source link

The prerender directive is included twice in the same page (part 1) #447

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Preparation for the exclusion of the prerender directive from the bottom 
navigation links (it is included twice).

Patch for /templates/diff.html

Original issue reported on code.google.com by phist...@chromium.org on 1 Jun 2013 at 4:31

Attachments:

GoogleCodeExporter commented 9 years ago
Part 2 is (not a well formed patch, the editing feature is failing on me with a 
400 error for some reason) -
--- a/templates/file_navigation.html
+++ b/templates/file_navigation.html
    {%if patch.next%}
+   {%if not exclude_prerender%}
        <link rel="prerender"
           href="{%ifequal view_style 'patch'%}{%url codereview.views.patch issue.key.id,patchset.key.id,patch.next.key.id%}{%else%}{%url codereview.views.diff issue.key.id,patchset.key.id,patch.next.filename%}{%endifequal%}{%urlappend_view_settings%}"></link>
+   {%endif%}
        <a id="nextFile"

Original comment by phist...@chromium.org on 1 Jun 2013 at 4:36

GoogleCodeExporter commented 9 years ago
Please upload patches to codereview.appspot.com.

To give a short feedback: I don't see where exclude_prerender comes from. I 
can't find this variable neither in the default branch nor in the chromium 
branch.

Original comment by albrecht.andi on 1 Jun 2013 at 4:40

GoogleCodeExporter commented 9 years ago
It does not come from anywhere, it is defined there exactly for this purpose.

Original comment by phist...@chromium.org on 1 Jun 2013 at 4:42

GoogleCodeExporter commented 9 years ago
Also... if you do not want patches this way, why are you allowing the edit file 
feature?

Original comment by phist...@chromium.org on 1 Jun 2013 at 4:46

GoogleCodeExporter commented 9 years ago
The patch was uploaded -
https://codereview.appspot.com/12351044/

Original comment by phist...@chromium.org on 2 Aug 2013 at 8:47