Docserver: Modify DocumentRenderer to generate absolute links

chrome/common/extensions/docs/server2/document_renderer.py

 # Copyright 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import logging
 import os
 from document_parser import ParseDocument
 from platform_util import ExtractPlatformFromURL
 from third_party.json_schema_compiler.model import UnixName
 
 
 class DocumentRenderer(object):
   '''Performs document-level rendering such as the title, references,
   and table of contents: pulling that data out of the document, then
   replacing the $(title), $(ref:...) and $(table_of_contents) tokens with them.
 
   This can be thought of as a parallel to TemplateRenderer; while
   TemplateRenderer is responsible for interpreting templates and rendering files
   within the template engine, DocumentRenderer is responsible for interpreting
   higher-level document concepts like the title and TOC, then performing string
   replacement for them. The syntax for this replacement is $(...) where ... is
   the concept. Currently title and table_of_contents are supported.
   '''
 
   def __init__(self, table_of_contents_renderer, platform_bundle):
     self._table_of_contents_renderer = table_of_contents_renderer
     self._platform_bundle = platform_bundle
 
-  def _RenderLinks(self, document, path):
+  def _RenderLinks(self, document, host, path):
     ''' Replaces all $(ref:...) references in |document| with html links.
 
         References have two forms:
 
           $(ref:api.node) - Replaces the reference with a link to node on the
                             API page. The title is set to the name of the node.
 
           $(ref:api.node Title) - Same as the previous form, but title is set
                                   to "Title".
     '''
@@ skipping 32 matching lines @@
         if platform is None:
           logging.error('Cannot resolve reference without a platform.')
           continue
         ref_dict = self._platform_bundle.GetReferenceResolver(
             platform).SafeGetLink(ref_parts[0],
                                   namespace=api_name,
                                   title=title,
                                   path=path)
 
         new_document.append(document[cursor_index:start_ref_index])
-        new_document.append('<a href=%s>%s</a>' % (ref_dict['href'],
-                                                   ref_dict['text']))
+        new_document.append('<a href=%s/%s/%s>%s</a>' % (host,

[not at google - send to devlin, 2014/08/06 23:00:48]

you shouldn't need a host, just /some/link will default to
http[s]://<host>/some/link

[ahernandez, 2014/08/07 00:56:12]

This complains about links with '/' at the
front:https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/extensions/docs/server2/patch_servlet_test.py&q=patch_servlet_test.py&sq=package:chromium&type=cs&l=34

This check only happens in the test file, but I'm a bit wary of changing the
check, particularly because I'm not sure why it checks for '/' at the beginning
of the link in the first place. There's no documentation explaining why a link
of that form is considered unqualified.

[not at google - send to devlin, 2014/08/07 15:21:18]

yeah, the leading / validity check is to catch bugs caused by inconsistencies.
you're right not to change it - but it should be possible to generate an
absolute link without triggering that? we're just generating HTML at this point,
not valid links.

what I mean is - if you take the |host| argument out of this phrase and just
change it to '<a href=/%s/%s>%s</a>' % (platform, ...) - will that work?

[ahernandez, 2014/08/07 17:21:31]

Changing it to '<a href=/%s/%s>%s</a>' is what I'm talking about: that produces
hrefs with a leading '/'.

[not at google - send to devlin, 2014/08/07 17:37:08]

such links are valid, at least on the web. I should have read your comment first
- that the patch servlet test is failing. i think you should fix the patch
servlet test to not crash :)

+                                                         platform,
+                                                         ref_dict['href'],
+                                                         ref_dict['text']))
 
       cursor_index = end_ref_index + 1
       start_ref_index = document.find(START_REF, cursor_index)
 
     new_document.append(document[cursor_index:])
 
     return ''.join(new_document)
 
-  def Render(self, document, path, render_title=False):
+  def Render(self, document, host, path, render_title=False):
+    ''' |document|: document to be rendered.
+            |host|: server the document is requested from.
+            |path|: request path to the document.
+    |render_title|: boolean representing whether or not to render a title.
+    '''
     # Render links first so that parsing and later replacements aren't
     # affected by $(ref...) substitutions
-    document = self._RenderLinks(document, path)
+    document = self._RenderLinks(document, host, path)
 
     parsed_document = ParseDocument(document, expect_title=render_title)
     toc_text, toc_warnings = self._table_of_contents_renderer.Render(
         parsed_document.sections)
 
     # Only 1 title and 1 table of contents substitution allowed; in the common
     # case, save necessarily running over the entire file.
     if parsed_document.title:
       document = document.replace('$(title)', parsed_document.title, 1)
     return (document.replace('$(table_of_contents)', toc_text, 1),
             parsed_document.warnings + toc_warnings)