560008 - Doc URLs should allow fragment identifiers

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: Documentation, enhancement)

Description

[Drew Willcoxon :adw]

It would be nice to able to link to a section within a page, like <a href="#subsection">subsection</a>.  So instead of requiring the user to find the subsection himself, we could link him directly to it, especially when it's referenced in several places.

Comment 1

[Atul Varma [:atul]]

Agreed. This will be a bit tricky, though, since we already use the fragment identifier as a sort of extension to the URI path, so we might need to have it be in a certain style such as "#subsection/blah" and possibly even have the JS loading the content rewrite it as something else.

We should also create some sort of convention that allows one module/package's docs to reference another, so that e.g. when the file module references ByteWriter objects, the reader can click on "ByteWriter" and be taken straight to its docs.

Comment 2

[Atul Varma [:atul]]

We may also want to auto-generate TOCs based on the different sections in the docs.

Comment 3

[Drew Willcoxon :adw]

(In reply to comment #2)
> We may also want to auto-generate TOCs

Filed bug 560735.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 5

[Will Bamberg [:wbamberg]]

Attachment: Patch to make doc pages complete pages

Here's a patch that makes SDK doc pages into separate pages, enabling us to use fragment identifiers.

This patch also moves almost all the doc generation into the server, getting us 90% of the way to being able to generate static pages for AMO, that are identical to the current pages, using 'cfx sdocs'.

Please let me know whether you think this is a good direction.

There are a few issues that need to be resolved, and I'd be happy to get feedback on those too.

(1) links: I've represented all links here as absolute paths from the server root. Although this works fine here, it's not clear to me how it would work if we were hosting these pages on AMO (or locally as file:// links for that matter). I suppose we could supply a root to cfx sdocs and have it rewrite all the links, but that's the kind of thing you'd really rather do in jQuery than Python. Maybe it would be better to rewrite links in the browser then?

(2) ping: the pinging mechanism doesn't work too well any more. Problem is that whenever you click a link, you lose the previous context, so you see an error in the debug output as it tries to respond to the AJAX request from the old context. I think pinging is probably needed in the local-server model, because you do need to know when you can shut the server down. I wondered if maybe you could still ping, and have the server treat it as a keepalive, but have the server not respond? But this code looks a bit intricate so I decided to ask, before rewriting it.

(3) asides: I'm not sure what to do about asides. They would need reimplementing to work on the server, and are broken anyway by images. Personally I don't think they are worth it and would vote to retire them.

(4) package detail pages: I'm wondering if we should remove the 'Authors' line? In the current code it's the concatenation of "Author' and 'Contributors', but 'Contributors' is evidently not kept up to date. We already have 'Credits' which we actively keep up to date.

It feels kind of slow. Is that just because we're loading a new page each time?

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 508064 [details] [diff] [review]
Patch to make doc pages complete pages

> (1) links: I've represented all links here as absolute paths from the server
> root. Although this works fine here, it's not clear to me how it would work if
> we were hosting these pages on AMO (or locally as file:// links for that
> matter). I suppose we could supply a root to cfx sdocs and have it rewrite all
> the links, but that's the kind of thing you'd really rather do in jQuery than
> Python. Maybe it would be better to rewrite links in the browser then?

If one supplies a root to cfx sdocs, could it simply add a <base> tag to pages rather than rewriting links?  That might be the easiest and most reliable absolute path option.


> (2) ping: the pinging mechanism doesn't work too well any more. Problem is that
> whenever you click a link, you lose the previous context, so you see an error
> in the debug output as it tries to respond to the AJAX request from the old
> context. I think pinging is probably needed in the local-server model, because
> you do need to know when you can shut the server down. I wondered if maybe you
> could still ping, and have the server treat it as a keepalive, but have the
> server not respond? But this code looks a bit intricate so I decided to ask,
> before rewriting it.

I'm not that familiar with the code, but this seems reasonable to me.  The ideal solution is probably to make pinging unnecessary, either by serving the files from file:// URLs or by leaving the server running until manually killed (i.e. making "cfx docs" block until killed by Ctrl-C; it could still be run in the background, of course, via "cfx docs &").


> (3) asides: I'm not sure what to do about asides. They would need
> reimplementing to work on the server, and are broken anyway by images.
> Personally I don't think they are worth it and would vote to retire them.

I like their style, but I just reviewed the ones that are in the SDK currently, and they all seem relatively easy to include as in-line notes à la those on MDC, f.e. in its XMLHttpRequest documentation <https://developer.mozilla.org/en/xmlhttprequest>, so I'm fine with this.


> (4) package detail pages: I'm wondering if we should remove the 'Authors' line?
> In the current code it's the concatenation of "Author' and 'Contributors', but
> 'Contributors' is evidently not kept up to date. We already have 'Credits'
> which we actively keep up to date.

Urk, unmaintained attributions are worse than no attributions.  And it feels a little strange to maintain a list of attributions by package, given that people contribute across packages, to non-package code, etc.

Sometimes I wonder if we should move Credits to a wiki page to make it even easier to keep up-to-date.

I also wonder if we should expand Credits to include brief summaries of the contributions the listed contributors have made.


> It feels kind of slow. Is that just because we're loading a new page each time?

I tried applying the patch, but it looks like it's a bit rotted.  Can you upload a new one that applies to the latest trunk?

Comment 7

[Will Bamberg [:wbamberg]]

Attachment: Updated patch against master

(In reply to comment #6)
> Comment on attachment 508064 [details] [diff] [review]
> Patch to make doc pages complete pages
> 
> > (1) links: I've represented all links here as absolute paths from the server
> > root. Although this works fine here, it's not clear to me how it would work if
> > we were hosting these pages on AMO (or locally as file:// links for that
> > matter). I suppose we could supply a root to cfx sdocs and have it rewrite all
> > the links, but that's the kind of thing you'd really rather do in jQuery than
> > Python. Maybe it would be better to rewrite links in the browser then?
> 
> If one supplies a root to cfx sdocs, could it simply add a <base> tag to pages
> rather than rewriting links?  That might be the easiest and most reliable
> absolute path option.
> 

That's a much better plan.

> > (3) asides: I'm not sure what to do about asides. They would need
> > reimplementing to work on the server, and are broken anyway by images.
> > Personally I don't think they are worth it and would vote to retire them.
> 
> I like their style, but I just reviewed the ones that are in the SDK currently,
> and they all seem relatively easy to include as in-line notes à la those on
> MDC, f.e. in its XMLHttpRequest documentation
> <https://developer.mozilla.org/en/xmlhttprequest>, so I'm fine with this.

Hm. Well, I'll see what it would take to rescue them. To be honest, I do agree that they work well, and I don't have a good reason for retiring them other than 'keeping them would be some work'.

> > (4) package detail pages: I'm wondering if we should remove the 'Authors' line?
> > In the current code it's the concatenation of "Author' and 'Contributors', but
> > 'Contributors' is evidently not kept up to date. We already have 'Credits'
> > which we actively keep up to date.
> 
> Urk, unmaintained attributions are worse than no attributions.  And it feels a
> little strange to maintain a list of attributions by package, given that people
> contribute across packages, to non-package code, etc.

Agreed. I felt the same about 'version' actually: the version listed for each package is actually the version of the SDK.

> Sometimes I wonder if we should move Credits to a wiki page to make it even
> easier to keep up-to-date.
> 
> I also wonder if we should expand Credits to include brief summaries of the
> contributions the listed contributors have made.

Shall I raise a bug for this?

> I tried applying the patch, but it looks like it's a bit rotted.  Can you
> upload a new one that applies to the latest trunk?

Done. It's also at https://github.com/wbamberg/jetpack-sdk/tree/560008 if that works better.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 511063 [details] [diff] [review]
Updated patch against master

(In reply to comment #7)
> Shall I raise a bug for this?

Yes, please!


> > I tried applying the patch, but it looks like it's a bit rotted.  Can you
> > upload a new one that applies to the latest trunk?
> 
> Done. It's also at https://github.com/wbamberg/jetpack-sdk/tree/560008 if that
> works better.

Thanks!  I'm not sure yet whether Git branches work better than Bugzilla patch attachments for me as a reviewer, and I can handle both, so generally I suggest that developers use whatever method works better or them.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 511063 [details] [diff] [review]
Updated patch against master

The asides are meant to be read after the texts they sit aside, but now that they appear inline, they show up before those texts, presumably because they are no longer floated to the right, which is what used to cause them to appear aside their texts. So their position will need to change in the source, from before their texts to after them.


The "cfx sdocs" code will need to write those <base> tags we discussed earlier in order for Zandr to be able to push the docs to mozillalabs.com, since they will be served from a subdirectory of the web server root directory there (i.e. /sdk/1.0b3/docs/).


diff --git a/packages/addon-kit/docs/notifications.md b/packages/addon-kit/docs/notifications.md

> -the [`self`](#module/api-utils/self) module documentation for more information.)
> +the [`self`](/packages/api-utils/docsself) module documentation for more information.)
...
> -    a data URI, or a URL returned by the [`self`](#module/api-utils/self)
> +    a data URI, or a URL returned by the [`self`](/packages/api-utils/docsself)

docsself -> docs/self


diff --git a/packages/addon-kit/docs/page-mod.md b/packages/addon-kit/docs/page-mod.md

> -[match-pattern](#module/api-utils/match-pattern) syntax.
> +[match-pattern](/packages/api-utils/docsmatch-pattern) syntax.
...
> -    [match-pattern](#module/api-utils/match-pattern) module for
> +    [match-pattern](/packages/api-utils/docsmatch-pattern) module for
...
> -A [list](#module/api-utils/list) of match pattern strings.  These define the
> +A [list](/packages/api-utils/docslist) of match pattern strings.  These define the
>  pages to which the page mod applies.  See the
> -[match-pattern](#module/api-utils/match-pattern) module for a description of
> +[match-pattern](/packages/api-utils/docsmatch-pattern) module for a description of

docsmatch-pattern -> docs/match-pattern
docslist -> docs/list


diff --git a/packages/api-utils/docs/file.md b/packages/api-utils/docs/file.md

-  [`text-streams`](#module/api-utils/text-streams) and
-  [`byte-streams`](#module/api-utils/byte-streams) for more information.
> +  [`text-streams`](/packages/api-utils/docstext-streams) and
> +  [`byte-streams`](/packages/api-utils/docsbyte-streams) for more information.

docstext-streams -> docs/text-streams
docsbyte-streams -> docs/byte-streams


diff --git a/python-lib/cuddlefish/server.py b/python-lib/cuddlefish/server.py

> +import markdown

Is the markdown module supposed to work with our current minimally-supported version of Python (2.5)?  I get an exception when I try to import it using that version, although it works fine with the default version of Python on my system (2.6):

  (addon-sdk)myk@myk:~/Projects/addon-sdk$ python2.5
  Python 2.5.4 (r254:67916, Jan 20 2010, 21:44:03) 
  [GCC 4.4.1] on linux2
  Type "help", "copyright", "credits" or "license" for more information.
  >>> import markdown
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    File "/home/myk/Projects/addon-sdk/python-lib/markdown/__init__.py", line 49, in <module>
      import logging
  ImportError: No module named logging
  >>>
  (addon-sdk)myk@myk:~/Projects/addon-sdk$ python --version
  Python 2.6.5
  (addon-sdk)myk@myk:~/Projects/addon-sdk$ python
  Python 2.6.5 (r265:79063, Apr 16 2010, 13:09:56) 
  [GCC 4.4.3] on linux2
  Type "help", "copyright", "credits" or "license" for more information.
  >>> import markdown
  >>> 


diff --git a/python-lib/cuddlefish/tests/static-files/packages/aardvark/docs/aardvark-feeder.md b/python-lib/cuddlefish/tests/static-files/packages/aardvark/docs/aardvark-feeder.md

> +@param food {string}
> +  The food.  Aardvarks will eat anything.

As long as it's a string? ;-)


diff --git a/static-files/index.html b/static-files/index.html

> +  <base target="_self"/>

Is this really necessary?  "_self" is the default value for "target", so omitting this should work the same.

Comment 10

[Will Bamberg [:wbamberg]]

Thanks Myk.
 
> The asides are meant to be read after the texts they sit aside, but now that
> they appear inline, they show up before those texts, presumably because they
> are no longer floated to the right, which is what used to cause them to appear
> aside their texts. So their position will need to change in the source, from
> before their texts to after them.

It turns out that it's simple to do something very much like the original asides using only CSS, so I'll do that.

> diff --git a/python-lib/cuddlefish/server.py b/python-lib/cuddlefish/server.py
> 
> > +import markdown
> 
> Is the markdown module supposed to work with our current minimally-supported
> version of Python (2.5)?  I get an exception when I try to import it using that
> version, although it works fine with the default version of Python on my system
> (2.6):
> 
>   (addon-sdk)myk@myk:~/Projects/addon-sdk$ python2.5
>   Python 2.5.4 (r254:67916, Jan 20 2010, 21:44:03) 
>   [GCC 4.4.1] on linux2
>   Type "help", "copyright", "credits" or "license" for more information.
>   >>> import markdown
>   Traceback (most recent call last):
>     File "<stdin>", line 1, in <module>
>     File "/home/myk/Projects/addon-sdk/python-lib/markdown/__init__.py", line
> 49, in <module>
>       import logging
>   ImportError: No module named logging
>   >>>
>   (addon-sdk)myk@myk:~/Projects/addon-sdk$ python --version
>   Python 2.6.5
>   (addon-sdk)myk@myk:~/Projects/addon-sdk$ python
>   Python 2.6.5 (r265:79063, Apr 16 2010, 13:09:56) 
>   [GCC 4.4.3] on linux2
>   Type "help", "copyright", "credits" or "license" for more information.
>   >>> import markdown
>   >>> 

That's worrying. It is certainly supposed to support Python 2.5:

http://www.freewisdom.org/projects/python-markdown/News 
"Python-Markdown supports Python versions 2.3, 2.4, 2.5, and 2.6."

It looks like it complains about not finding the 'logging' module, but logging is present in Python since 2.3: http://docs.python.org/library/logging.html

I don't get this error, on OS X or Linux. I get the expected:

~/mozilla/jetpack-sdk/python-lib(560008) > python2.5
Python 2.5.4 (r254:67916, Jun 24 2010, 21:47:25) 
[GCC 4.2.1 (Apple Inc. build 5646)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import markdown
>>> markdown.markdown("**anything**")
u'<p><strong>anything</strong></p>'
>>> exit()

Does 'import logging' on its own also fail?

Comment 11

[Will Bamberg [:wbamberg]]

Attachment: Updated patch

Updated patch, also pushed to: https://github.com/wbamberg/jetpack-sdk/tree/560008

With respect to asides and python-markdown compatibility, please see comment 10.

So cfx sdocs has a new argument:

    cfx --baseurl='/Users/mozilla/web-docs/' sdocs

...will insert '/Users/mozilla/web-docs/' in the <base> tag of every page generated:

    <base href="/Users/mozilla/web-docs/" />

All other internal links are made relative to that. I hope this works well enough for both Zandr and Piotr, who I guess are the main (only?) customers of cfx sdocs. It is a shame, I think, that the output of sdocs isn't portable any more.

> > (2) ping: the pinging mechanism doesn't work too well any more. Problem is that
> > whenever you click a link, you lose the previous context, so you see an error
> > in the debug output as it tries to respond to the AJAX request from the old
> > context. I think pinging is probably needed in the local-server model, because
> > you do need to know when you can shut the server down. I wondered if maybe you
> > could still ping, and have the server treat it as a keepalive, but have the
> > server not respond? But this code looks a bit intricate so I decided to ask,
> > before rewriting it.
> 
> I'm not that familiar with the code, but this seems reasonable to me.

I've implemented this for the time being. It seems to work well, and the code looks to me like it should work, but I don't quite understand what the original code in server.py ( the 'elif parts[0] == IDLE_PATH:' branch inside '_respond_with_api') was doing, so I am a bit worried about that.

I would like to tweak the style for the title "Add-on SDK" so it floats right of the logo and wraps, which will stop it being pushed under the logo on a small-width or low-resolution browser window. Generally I'd like to improve the appearance of the docs on lower resolution screens.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #10)
> Does 'import logging' on its own also fail?

It does!  So it sounds like the problem is in my Python 2.5 installation.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #12)
> (In reply to comment #10)
> > Does 'import logging' on its own also fail?
> 
> It does!  So it sounds like the problem is in my Python 2.5 installation.

Indeed, my installation comes from Ubuntu/Debian's python2.5-minimal package, which "contains the interpreter and some essential modules.  It can
be used in the boot process for some basic tasks."  A look at the list of modules included with the installation reveals that it doesn't come with 'logging'.

Comment 14

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 511814 [details] [diff] [review]
Updated patch

diff --git a/packages/addon-kit/docs/page-mod.md b/packages/addon-kit/docs/page-mod.md

  -![Multiple workers](media/multiple-workers.jpg)
  +![Multiple workers](/media/multiple-workers.jpg)

This change breaks display of the image when the docs are served from a non-root directory.  Now that you're writing <base> tags, simply undoing the change should fix things.


diff --git [various files]

  -[Working with Content Scripts](#guide/addon-development/web-content) for more
  +[Working with Content Scripts](/dev-guide/addon-development/web-content) for more

There are three occurrences of (/dev-guide/addon-development/web-content) in various files, which has two problems: the leading slash breaks the link when the docs are served from a non-root directory, and the filename should have ".html" appended to it.  The correct link (which appears in 14 other places) is: (dev-guide/addon-development/web-content.html).


diff --git a/static-files/index.html b/static-files/index.html

The role of this file is no longer clear to me (and the changes to it are stale).  Can it simply be removed?


diff --git a/static-files/md/dev-guide/addon-development/xul-extensions.md b/static-files/md/dev-guide/addon-development/xul-extensions.md

All the links in this file have the "leading slash" and "no trailing .html" problems.


diff --git a/static-files/md/dev-guide/module-development/best-practices.md b/static-files/md/dev-guide/module-development/best-practices.md

     [Security Roadmap]: #guide/security-roadmap

This link would need to be updated, except that the security roadmap doc doesn't exist (I think it was removed at some point), so the better option is to remove the reference to it.


diff --git a/static-files/md/dev-guide/module-development/chrome.md b/static-files/md/dev-guide/module-development/chrome.md

   When the manifest implementation is complete (see the
  -[Security Roadmap](#guide/security-roadmap) for details), the runtime loader
  +[Security Roadmap](dev-guide/security-roadmap) for details), the runtime loader
   will actually prevent modules from `require()`ing modules that are not listed

Ditto.


Otherwise good!  And close!

Comment 15

[Will Bamberg [:wbamberg]]

Attachment: Fix those links

Comment 16

[Will Bamberg [:wbamberg]]

> diff --git a/static-files/md/dev-guide/addon-development/xul-extensions.md
> b/static-files/md/dev-guide/addon-development/xul-extensions.md

This doc doesn't make a lot of sense any more. I've tried editing it so it's not obviously wrong, but that's probably worse, as it's now subtly wrong. 

It seems actually as if the technique described here doesn't currently work (http://asqueella.blogspot.com/2010/03/jetpack-sdk-and-xul-extensions.html) so probably the best thing to do for this release would be to add a note to the top explaining that it's not currently working and pointing at Nickolay's work.

Comment 17

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 512374 [details] [diff] [review]
Fix those links

(In reply to comment #16)
> It seems actually as if the technique described here doesn't currently work
> (http://asqueella.blogspot.com/2010/03/jetpack-sdk-and-xul-extensions.html) so
> probably the best thing to do for this release would be to add a note to the
> top explaining that it's not currently working and pointing at Nickolay's work.

Sure.  Separate patch?

Comment 18

[Will Bamberg [:wbamberg]]

Fixed by: https://github.com/mozilla/addon-sdk/commit/5512c7a143bfeac2cc9bbcffda744e8cd4c5cbd4

and:

https://github.com/mozilla/addon-sdk/commit/a6fb272dc910664525b7f6c0babdd48a5ce86bb2