idmillington / undum

A client-side framework for narrative hypertext interactive fiction.
https://idmillington.github.com/undum
MIT License
336 stars 80 forks source link

Fix bug with augmentLinks() not working properly when doWrite inserts an a element #39

Closed sequitur closed 5 years ago

sequitur commented 9 years ago

I noticed this issue while working on a project: I had set up a system (part of a miniature helper library) to automate generating "cycling links" similar to the popular Twine extension, and found that they weren't working. After tracking down the bug, I found why: If doWrite() is called to insert an a element, augmentLinks doesn't work. The culprit turned out to be $.find; it only traverses the children of the matched elements. So for instance, if you pass the following html to doWrite():

<a id="outer" href="./outer">
Link <a id="inner" href="./inner">Text</a>
</a>

Only #inner will be augmented correctly, because when $.find() is called on this block of html it returns only that element. Fixing this issue is simple: We compose $.find() with $.addBack(), which goes back one step of JQuery's DOM traversal stack, filters it (In this case once again looking for a elements), and adds them back into the chain. This means that top-level elements are also matched and correctly augmented; so if you want to insert a whole pile of inline anchors and spans nested and interspersed, you can:

<a>
  <span>
    <a class="inner">
      stuff
    </a>
  </span>
</a>
<span>
  and things
</span>
<span>
  and
  <a>
    more
  </a>
</span>

...will have all of its anchors, and only them, correctly matched and augmented inside augmentLinks().

idmillington commented 5 years ago

This seems super fragile because it depends on where augmentLinks was called from. It works in the current code, but if it failed to work in the future it would be a mysterious bug. Is there a way to be more explicit?