- Issue created by @azinck
- πΊπΈUnited States azinck
I'm going to provide a fix that works for my needs, but I recognize that it's not a true fix for this module because it doesn't support many of the use-cases the module tries to support. There is a fundamental problem with the way that numbering, linking, and counting is managed in the module at present and I don't have a deep enough understanding of the things the module's trying to do to sort through the right solution right now.
A key part of this bug stems from the logic in \Drupal\footnotes\FootnotesGroup::add that avoids adding a footnote if it's a textual duplicate of a footnote that was added during a previous call to the same method. I honestly don't know why that logic exists. At the time it's called from the text processor the link in the text to the footnote has already been created, but now it's just nuking the footnote that the link references, breaking the link. It operates regardless of the status of the "Collapse footnotes with identical content" setting, but even if it were to be adjusted to honor that setting I think there are still problems, but I'm not 100% sure...I know there's some JS the module provides that tries to fix some footnote numbering in some of these situations so maybe that handles it, but this is where my lack of familiarity with the module comes in.
Anyhow, another outcome of that problem is that now we have a problem with our footnote numbering if the \Drupal\footnotes\FootnotesGroup::count is ever called again (which will happen for the next text field that gets processed). The count method just looks at the number of entries in the matches array, which is actually misaligned from the footnote numbers that are in the text on the page at this point.
Anyhow, my fix here is a blunt instrument. I don't care if the content of my footnotes matches. I want all the footnotes on the page, so this patch removes the text matching logic in \Drupal\footnotes\FootnotesGroup::add.
If someone with more familiarity with the nuances of how the collapsing logic is meant to work can weigh in and plot a path forward here I'd be happy to update the patch to be smarter.
- π¬π§United Kingdom scott_euser
The non-JS option will always struggle in some scenarios because the only way a layout builder/experience builder/paragraphs based content can build up the combined footnotes as is is when they get rendered (or at least currently). You may get more mileage with the JS based grouping instead as the numbering, etc is all calculated after the page fully renders including getting recalculated if new elements arrive on the page (e.g. via bigpipe or via ajax).
But yeah if you did ever want something to get in:
- Needs to be via merge request so coding standards and test coverage gets run
- Needs to have test coverage run for the change
- If the change could potentially negatively affect existing sites, needs to have an opt-in feature
- πΊπΈUnited States matthand Riverdale Park, Maryland
Seeing the same bug. When footnote text is the same it forces a grouping and breaks things. Regardless of how the configuration checkbox is set in the text filter.
- πΊπΈUnited States matthand Riverdale Park, Maryland
Issued a MR with a change to check the collapse footnotes setting when adding footnotes to a group.
- π¬π§United Kingdom scott_euser
Thanks both for figuring out the problem and working out solution! Looks sensible
We should also add to the tests so it stays fixed. Ie, adding an example of two identical texts getting collapsed & not collapsed to FootnotesGroupBlockTest for example. FootnotesFilterPluginTest::testCollapseFootnotes() already has this outside of the block context, so the issue here is no coverage for within the Group block.
- πΊπΈUnited States azinck
Here's the reason I didn't solve this using this approach in my initial patch: I'm not convinced it's right. It's definitely the correct approach if you don't want to collapse footnotes, but for people who have chosen "Collapse footnotes with identical content" how does this work correctly?
What is the intended behavior when "Collapse footnotes with identical content" is selected? Let's take an example where we have 3 footnotes where the footnotes contain the content: "A", "A", and "B", respectively.
When "Collapse footnotes" is selected do we expect to see our content annotated with [1], [1], [2] (since we have 2 instances of footnote 1)? Or with [1], [2], [3] where [1] and [2] both link to a single line in the footnotes block that reads something like [1],[2]: "A"?
- π¬π§United Kingdom scott_euser
When "Collapse footnotes" is selected do we expect to see our content annotated with [1], [1], [2] (since we have 2 instances of footnote 1)? Or with [1], [2], [3] where [1] and [2] both link to a single line in the footnotes block that reads something like [1],[2]: "A"?
[1], [1], [2] in that case and that's what FootnotesFilterPluginTest::testCollapseFootnotes() covers. Group is essentially an extension of that to handle multiple formatted texts on the same page (and the JS version, multiple on the same page with lazy loading/ajax loaded content). The thing not part of offline medium is the backlinks since linking back to source references is not a thing offline.
Ultimately the module has to take some opinionated decisions, and if people aren't happy with those opinions, needs to be flexible enough to allow control - but that of course adds complexity, hence the push always for more test coverage or it will end up being impossible for me to merge things confidently, so that was one of the bigger chunks of work when building out the current version. Some footnotes style guides for print media will say [1], [2], [3] and converting [2] to use `Ibid` when its a repeat of [1] - something I can't imagine being easy with the reference being formatted text, but anyways if someone wants to contribute that it'd be a separate feature needing its own test coverage.
- πΊπΈUnited States azinck
Thanks for the explanation. Is the collapsing functionality meant to work at all without JS? I'm asking about not using JS because this is on a site I've inherited that has dramatically changed the template markup for the footnotes block in a way that breaks the JS functionality, so I've been mostly looking at this from a non-JS perspective. Because if it *is* meant to work without JS I'm confused as to *how* it would work, either with or without the fixes in this issue.
- π¬π§United Kingdom scott_euser
Yes meant to work, probably an oversight when Group functionality was merged (was a very long standing issue with many contributors over years) https://www.drupal.org/project/footnotes/issues/3098138 π All footnotes available as block for Layout Builder / Paragraph / ... Fixed though I haven't revised that deeply to check.
So each footnote text has a hash of the text in the ID. The ID is unique but when stripped to get just the hash bit, if the hash matches, then the citation should target the same reference as already stored for output. If the hash doesn't exist the citation should cause a new reference to be created.