Extension docs: Include sidenav in 404 pages

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

Index: chrome/common/extensions/docs/server2/sidenav_data_source.py
===================================================================
--- chrome/common/extensions/docs/server2/sidenav_data_source.py	(revision 201930)
+++ chrome/common/extensions/docs/server2/sidenav_data_source.py	(working copy)
@@ -14,17 +14,21 @@
   menu.
   """
   class Factory(object):
-    def __init__(self, compiled_fs_factory, json_path):
+    def __init__(self, compiled_fs_factory, json_path, base_path):
       self._cache = compiled_fs_factory.Create(self._CreateSidenavDict,
                                                SidenavDataSource)
       self._json_path = json_path
+      self._base_path = base_path
 
     def Create(self, path):
       """Create a SidenavDataSource, binding it to |path|. |path| is the url
       of the page that is being rendered. It is used to determine which item
       in the sidenav should be highlighted.
       """
-      return SidenavDataSource(self._cache, self._json_path, path)
+      return SidenavDataSource(self._cache,
+                               self._json_path,
+                               path,
+                               self._base_path)
 
     def _AddLevels(self, items, level):
       """Levels represent how deeply this item is nested in the sidenav. We
@@ -40,10 +44,12 @@
       self._AddLevels(items, 2);
       return items
 
-  def __init__(self, cache, json_path, path):
+  def __init__(self, cache, json_path, path, base_path):
     self._cache = cache
     self._json_path = json_path
     self._file_name = path.split('/')[-1]
+    self._doc_class = path.split('/')[0]
+    self._base_path = base_path

[not at google - send to devlin, 2013/05/28 14:53:37]

perhaps make this all:

self._dir_name, self._file_name = path.rsplit('/', 1)

see below.

[方觉(Fang Jue), 2013/05/28 23:37:50]

Suppose we want to add manifest/*.html (with the manifest generation patch) to
sidenav. What path should we use in extensions_sidenav.json?
"manifest/version.html"? If that's the case, this should be
self._doc_class, self._file_name = path.split('/', 1)
If it should be "version.html", the code you suggested is correct.

[not at google - send to devlin, 2013/05/28 23:45:27]

I see. It should be manifest/version.html, that makes sense. ok.

 
   def _AddSelected(self, items):
     for item in items:
@@ -56,8 +62,21 @@
           return True
     return False
 
+  def _ConvertToAbsolutePath(self, items):
+    for item in items:
+      file_name = item.get('fileName')
+      if file_name is not None and not file_name.startswith(('/',
+                                                           'http://',
+                                                           'https://')):
+        item['fileName'] = '%s/%s/%s' % (self._base_path,

[not at google - send to devlin, 2013/05/28 14:53:37]

'fileName' is actually wrong, it should have been called 'path' particularly
since they're not all file names (they might be directories, or external URLs).

I suppose it's a bit of make-work to change it though. oh well, you decide.
either way it obviously won't break anything due to the version pinning.

[方觉(Fang Jue), 2013/05/28 23:37:50]

*sidenav.json, sidenav_data_source and its tests, and private/sidenav* will need
changes I think. But 'fileName' is actually not a good name here. 'path' is not
a good name for external URLs? What about 'href'?

[not at google - send to devlin, 2013/05/28 23:45:27]

href makes sense.

+                                         self._doc_class,

[not at google - send to devlin, 2013/05/28 14:53:37]

doc class could be empty if there is no branch, which is legitimate. we don't
want to render all sidenav items in 'stable' as '/stable/extensions/foo.html'
because (a) that involves an extra redirect for every sidenav entry and (b) it
looks bad in tooltips and if copy-pasting links.

the logic should be something like:
- if the path is absolute, do nothing
- if the path is relative, prepend it with the "directory" of the requested
path.

I think that |path| is entirely enough to figure this out. basically I think you
need to take out doc_class and replace it with path.rsplit('/', 1)[0].

+                                         file_name)
+      if 'items' in item:
+        self._ConvertToAbsolutePath(item['items'])
+
   def get(self, key):
     sidenav = copy.deepcopy(self._cache.GetFromFile(
         '%s/%s_sidenav.json' % (self._json_path, key)))
     self._AddSelected(sidenav)
+    self._ConvertToAbsolutePath(sidenav)
     return sidenav