static-docs paths are broken for some images

RESOLVED FIXED

Status

Add-on SDK
Documentation
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: wbamberg, Assigned: wbamberg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
The fix for bug 708060 is buggy: in particular, it doesn't always rewrite the links to images properly, so in the static docs for the online version some images don't show up. Luckily I noticed this for the 1.5 doc set and patched it for those static docs, but this ought to be fixed.
(Assignee)

Comment 1

7 years ago
Created attachment 603115 [details] [diff] [review]
Make sure all img links are rewritten; test for it; make sure tags are escaped in quoted code

It seems as if Python's HTMLParser doesn't find IMG tags that are self-closed:

<img alt="my picture" src="path/to/my-picture"/>

...which is what the Markdown parser will emit when presented with syntax like:

![my picture](path/to/my-picture)

(Perhaps I have the wrong DOCTYPE for that to work?)

So when building static docs using cfx sdocs, those IMG links didn't get rewritten, and the images were then not found. A corresponding omission from the tests meant this wasn't easily discovered. I've extended the test to cover those cases (and stolen part of the test_licenses test to make the output more helpful).

I've also had to escape some "<" characters in IMG elements that were in quoted example code, so HTMLParser didn't find and rewrite those. Escaping just the IMG link in these cases didn't always work - the escaped character didn't get displayed. It seems as if I needed to escape all the preceding "<" characters to get it to work, although I don't understand why, which makes me uncomfortable with this fix...
Attachment #603115 - Flags: review?(warner-bugzilla)
Hm, that escaping-<-but-not-> bit does seem fishy.. is there a better way to do that? E.g. since normally HTML inside Markdown is supposed to be copied verbatim to the output HTML, and in this case we want it to appear escaped inside a <pre> block or something, maybe there's some better way to mark this HTML block? Markdown has a literal-code syntax, right?, like prefixing each line with four spaces or something?

It'd be nice if we could use the markdown syntax for the IMG tags, instead of needing to jump straight to full HTML.

I think I need to study the markdown translator a bit more.
(Assignee)

Comment 3

7 years ago
(In reply to Brian Warner [:warner :bwarner] from comment #2)
> Hm, that escaping-<-but-not-> bit does seem fishy.. is there a better way to
> do that? E.g. since normally HTML inside Markdown is supposed to be copied
> verbatim to the output HTML, and in this case we want it to appear escaped
> inside a <pre> block or something, maybe there's some better way to mark
> this HTML block? Markdown has a literal-code syntax, right?, like prefixing
> each line with four spaces or something?

It does, and this will result in markup like:

<pre>
  <code>
    blarg();
  </code>
</pre>

But this stuff also has to interact with the syntax highlighter. The syntax highlighter's usage rules are here: http://alexgorbatchev.com/SyntaxHighlighter/manual/installation.html. 

To make the common case simple, there's a bit of JS in the doc system: https://github.com/mozilla/addon-sdk/blob/master/doc/static-files/js/main.js#L7 that turns <pre><code> into the syntaxhighlighter's runes for JavaScript highlighting. This means that if you just mark something as code using Markdown, it's treated as JS by the highlighter.

But HTML and CSS need special treatment, and that's the reason for this weird stuff around those code samples (I did investigate http://softwaremaniacs.org/soft/highlight/en/, which gives very pretty highlighting, but does autodetection of the language, and seemed very unreliable to me).
(Assignee)

Comment 4

7 years ago
So, the problem with IMG links is solved: it was because my link-rewriting code wasn't paying attention to self-closing tags. Fixing that is simple, and means that Markdown IMG elements:

![my picture](path/to/my-picture)

...work fine.

The other problem is not solved. It seems to come from the interaction between the SyntaxHighlighter and Python's HTMLParser.

As this link: http://alexgorbatchev.com/SyntaxHighlighter/manual/installation.html explains, there are a couple of ways of using SyntaxHighlighter:

1) wrap the code inside <pre class="brush: whatever"></pre>
2) wrap the code inside a CDATA segment.

For HTML code I was using approach (2) (I think because I had problems with approach 1, but can't remember). But, Python's HTMLParser doesn't support CDATA - correctly, because CDATA is an XML thing, not an HTML thing: http://bugs.python.org/issue7114. So weird things happen.

So I've tried approach (1), but have found problems with that too. According to the SyntaxHighlighter's instructions, with approach (1) I have to escape all angled brackets. But when I do this, HTMLParser seems to strip out the escaped characters completely. So given markup like this:

<pre>
&lt;html&gt;
&lt;/html&gt;
</pre>

...the contents of that <pre> block as given by HTMLParser.handle_data() is:

html          
/html


Sigh. I've attached a very simple Python script to show what I mean.
(Assignee)

Updated

7 years ago
Attachment #603115 - Flags: review?(warner-bugzilla)
(Assignee)

Comment 5

7 years ago
Created attachment 603943 [details]
A simple script showing the problem with escaped characters
(Assignee)

Comment 6

7 years ago
Created attachment 604250 [details] [diff] [review]
Another attempt

OK, this patch uses the <pre class="brush: html"> approach to SyntaxHighlighter integration, escapes all "<" and ">" characters, and overrides HTMLParser.handle_entityref so as to preserve the escaping.

I still don't like it very much - the handle_entityref stuff seems pretty hacky to me - but at least I have some idea what it's doing now, and it seems to work.
Attachment #603115 - Attachment is obsolete: true
Attachment #604250 - Flags: review?(warner-bugzilla)
Comment on attachment 604250 [details] [diff] [review]
Another attempt

Yeah, I guess anytime you have HTML-inside-HTML you're going to run into some form of this problem. Come to think of it, some docs systems handle this by having file-interpolation/inclusion directives, where you have some markup that says "please include the contents of file X, in format Y, right here". That solves the language-A-in-language-B problem (i.e. having HTML inside your Markdown file and confusing your editor), and gives the translator enough information to know that the second file is supposed to be syntax-highlighted instead of interpreted directly.

But anyways, this one looks good.
Attachment #604250 - Flags: review?(warner-bugzilla) → review+

Comment 8

7 years ago
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/efd86a991e67eab310de0b88a308e3468b85e154
Bug 733098 - static-docs paths are broken for some images; r=@warner
(Assignee)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 9

6 years ago
Commit pushed to stabilization at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/7384d0e0209c4e781daf669dc3f029d0919e6102
Bug 733098 - static-docs paths are broken for some images; r=@warner
(cherry picked from commit efd86a991e67eab310de0b88a308e3468b85e154)
You need to log in before you can comment on or make changes to this bug.