Docserver: Modify DocumentRenderer to generate absolute links

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

Index: chrome/common/extensions/docs/server2/document_renderer.py
diff --git a/chrome/common/extensions/docs/server2/document_renderer.py b/chrome/common/extensions/docs/server2/document_renderer.py
index 6a033752a925fea7dac64505ead54444f7776001..9f6834b884e993bb51317ab18e93f31bac93c4df 100644
--- a/chrome/common/extensions/docs/server2/document_renderer.py
+++ b/chrome/common/extensions/docs/server2/document_renderer.py
@@ -26,7 +26,7 @@ class DocumentRenderer(object):
     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:
@@ -79,8 +79,10 @@ class DocumentRenderer(object):
                                   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)
@@ -89,10 +91,15 @@ class DocumentRenderer(object):
 
     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(