Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-unique HTML id attribute values in source-line references of code-blocks #4031

Closed
HazemAM opened this issue Nov 2, 2017 · 5 comments
Closed

Comments

@HazemAM
Copy link

HazemAM commented Nov 2, 2017

Pandoc's new sourceLine references in code-blocks (since 2.0) are using repeating HTML id attributes, which makes referencing some source-lines in a page that contains multiple code-blocks impossible (and it's also invalid HTML).

The problem

Exmaple input:

```xml
<file>SegoeUI-Light.ttf</file>
```

```xml
<file>Tahoma.ttf</file>
```

Output:

<pre class="sourceCode xml">
    <code class="sourceCode xml">
        <div class="sourceLine" id="1" href="#1" data-line-number="1">
            <span class="kw">&lt;file&gt;</span>SegoeUI-Light.ttf<span class="kw">&lt;/file&gt;</span>
        </div>
    </code>
</pre>

<pre class="sourceCode xml">
    <code class="sourceCode xml">
        <div class="sourceLine" id="1" href="#1" data-line-number="1">
            <span class="kw">&lt;file&gt;</span>Tahoma.ttf<span class="kw">&lt;/file&gt;</span>
        </div>
    </code>
</pre>

In this case, and if the code is hosted on http://thepage.com/, visiting http://thepage.com/#1 link will always reference the first line of the first block, and never the first line of the second block.

Another problem with this format is that IDs starting with a non-alphabet character ([a-zA-Z]) are not valid prior to HTML 5, and may not work on older browsers/renderers with no HTML 5 support. (I mention this since I see Pandoc having both html and html5 output formats).

Another point: Is there a reason there are href properties in sourceLines? Because it has no effect here, and referencing source lines will work with ids alone, and without hrefs (and it's also invalid HTML to have hrefs in divs).

Suggested solution

As a solution, I suggest prepending a block number to the ID (as in id="block1-line1", id="block2-line1"), which will also make the IDs more unique and understandable, and solves the problem of HTML 5 compatibility.

Format ideas:

  1. block1-line1
  2. b1-l1 (less understandable)
  3. line1-1 (as in line<block>-<line>)
  4. code-1.1

I would also suggest removing the href attributes from sourceLines.

Expected output (issue-irrelevant lines trimmed):

<div class="sourceLine" id="line1-1" data-line-number="1">
    <!-- some code -->
</div>

<!-- ... -->

<div class="sourceLine" id="line2-1" data-line-number="1">
    <!-- some code -->
</div>

More info

All this was tested using Pandoc 2.0.1 in a Windows 10 machine.

Sorry if this should go in jgm/skylighting. I thought about reporting this issue there, but thought maybe it's more relevant here since this is a document-level issue, not code-block-level issue. I hope I'm not wrong.

Thanks!

@jgm
Copy link
Owner

jgm commented Nov 2, 2017

I'd like to get thoughts on this from @dbaynard, who added these features in
jgm/skylighting#14

If href's are not allowed on the divs, then probably we shouldn't have them. But I think the intent was to allow you to click on a line and then copy a link to it from the top bar of the browser.

Since skylighting is independent of pandoc, we'd have to figure out how to get the unique identifiers in there.

@HazemAM
Copy link
Author

HazemAM commented Nov 2, 2017

If href's are not allowed on the divs, then probably we shouldn't have them.

Not only are they not allowed, hrefs won't have any effect except on <a> and <link> elements (and other less-popular ones: <area> and <base>), so they're useless here.


But I think the intent was to allow you to click on a line and then copy a link to it from the top bar of the browser.

Looking into the referenced pull, I see this:

If the linkAnchors option is set, wrap with an a element rather than a div, set so that clicking the line number (and only the line number) will jump to that line.

I think the idea was to replace the wrapping div with an a element, and then add the href attribute to it. So in case of linkAnchors, this:

<div class="sourceLine" id="1" data-line-number="1">
    <!-- some code -->
</div>

Would become this:

<a class="sourceLine" id="1" href="#1" data-line-number="1">
    <!-- some code -->
</a>

In that case, having an href attribute would be valid (with an a, not with a div) and will work as described (except for the "clicking the line number, and only the line number" part, because the whole line would be clickable in that case).

jgm added a commit to jgm/skylighting that referenced this issue Nov 2, 2017
jgm added a commit to jgm/skylighting that referenced this issue Nov 2, 2017
This is needed because a web page may contain several
code samples, and we need to make sure that the ids on
lines are unique.

See jgm/pandoc#4031.
@jgm jgm closed this as completed in 856587f Nov 2, 2017
@dbaynard
Copy link

dbaynard commented Nov 2, 2017

The "clicking the line number, and only the line number" is because links for the full line were disabled in css. From the code, I believe the multiple href issue pre-dated the recent changes, though, and this is a pandoc issue.

Would it be possible to use the uniqueIdent function for the ids?

@dbaynard
Copy link

dbaynard commented Nov 2, 2017

Also the changes seem reasonable. Please excuse the oversight.

@HazemAM
Copy link
Author

HazemAM commented Nov 3, 2017

@jgm - Thanks for the fix. I tested it using the nightly builds. I just wanted to point out that the data-line-number attribute also contain the id/cb# prefix. I'm not sure if this is intended, but this will make the line numbers appear prefixed in the output too:

Prefixed line numbers in code blocks

I would suggest adding the prefix only to the id attribute, and leaving the data-line-number un-prefixed, regardless of the block number:

<div class="sourceLine" id="cb3-1" data-line-number="1">
    <!-- some code -->
</div>

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants