Issues with multiple references to the same footnote

[jsma]

The current implementation assigns an ID to <sup> elements based on the footnote's index.

https://github.com/torchbox/wagtail-footnotes/blob/0a13e3757c8a435cd3add7131cd2d19fc9fb1cc8/wagtail_footnotes/blocks.py#L47

This leads to duplicate id values so that browsers (tested in Chrome and Firefox) will only return to the first occurrence in the copy. This can cause issues if the first instance of "[1]" is not currently visible (e.g. in a collapsed accordion), the browser won't do anything when the "Back to content" link is clicked.

I've started working on an implementation to fix this situation which borrows ideas from Wikipedia. They handle this by always assigning unique IDs and then have individual links back to each reference in the copy, using letters of the alphabet. If there is a single reference, then the caret is clickable, otherwise the caret is not clickable, only the letters are:

I have a basic WIP in my fork so that the block rendering will handle assigning unique IDs to the links and tracks these on the page object so when footnotes are rendered, they can be accessed.

This WIP does not currently change anything on the front-end. I'm still experimenting but so far can solve our issue by using a custom template and template tag to process multiple back links. For simplicity, I'm just using integers as the link text but it shouldn't be too hard to convert these to letters of the alphabet a la Wikipedia:

{% load core_tags %}
{% load wagtailcore_tags %}

{% if page.footnotes_list %}
  <ol>
    {% for footnote in page.footnotes_list %}
      <li id="footnote-{{ forloop.counter }}">
        {{ footnote.text|richtext }}
        {% with reference_ids=page|get_references:footnote.uuid %}
          {% with num=reference_ids|length %}
            {% if num == 1 %}
              <a href="#{{ reference_ids.0 }}" aria-label="Back to content">↩</a>
            {% else %}
              <span aria-label="Back to content">↩</span>
              {% for reference_id in reference_ids %}
                <a href="#{{ reference_id }}">
                  <sup>
                    <i><b>{{ forloop.counter }}</b></i>
                  </sup>
                </a>
              {% endfor %}
            {% endif %}
          {% endwith %}
        {% endwith %}
      </li>
    {% endfor %}
  </ol>
{% endif %}

And a simple template tag since we can't do variable lookups in dictionaries:

@register.filter
def get_references(value, footnote_uuid):
    return value.footnotes_references[footnote_uuid]

Let me know if something like this could be considered for inclusion. Happy to make a formal and comprehensive PR, but looking for feedback first. Thanks!

[nimasmi]

Thanks @jsma, this looks like a worthwhile addition. In particular, the use of duplicate IDs is a concern, so regardless of the additional features, the fact that this prevents editors from inadvertently creating invalid HTML is a good thing.

~I haven't seen I'd be concerned about the multiple calls to the |get_references filter, and wonder whether that can be moved outside the forloop, and converted to something that returns an iterable.~ Ah, I've just noticed you provide example code for that filter. A dict lookup could work fine if it's precomputed on the page.

[jsma]

A dict lookup could work fine if it's precomputed on the page.

Can you clarify? I haven't had my morning tea yet ;) The template tag was just a quick workaround for the limitation of the Django template language where you can't do a dictionary lookup using an attribute of another object e.g. page.footnotes_references.footnote.uuid.

[nimasmi]

Yes, sorry. I was instinctively wary at first when I saw your |get_references filter function, because I had imagined it performing queries every time it is called.

If it's just looking up items from a dict, which has previously been assembled e.g. when instantiating or .serve()ing the page, or similar, then it's fine. You haven't provided the example code for how value.footnotes_references is assembled, so I was just preemptively making sure it doesn't become an expensive, slow function.

As a worse example, a bad implementation might be if value.footnotes_references were a property method, which performed a query, and returned a dict (i.e. not precomputed by the page or the block).

Like I said, you haven't given any example implementation, so I'm sorry I doubted you, but better safe than sorry.

[jsma]

Ah, sorry if the link wasn't noticeable in my initial post but the implementation is here in my fork: https://github.com/GreenLightGo/wagtail-footnotes/pull/1/files

[jsma]

My previous comment about how Wikipedia handles this was too English-specific. For Chinese, Russian, and several other languages, they use integers rather than letters of the alphabet. This may be the best/only language-neutral option: