Hello,

I just realized that the latest version of footnotes has some problems when used with textile. Actually, there's one problem and a inconsistency regarding the code generated with the normal functioning mode (aka non-textile) and the textile-one.

1) The bug: your preg_match includes the newline character that should be before the 'footnotes.' keyword, however doesn't put it back. Therefore, the following code will generate a false output:
h2. Notes

footnotes.

will pass the following textile code to the the textile parser:
h2. Notes
fn1. I'm a footnote

This is simply corrected by appending a newline before the footer, which I did as you'll see in the provided patch. However this might not be the preferred place to do it, so you might want to move it elsewhere...

2) The footnotes when using the non-textile mode are printed within a div block with class footnotes (btw, in my humble opinion you could put an id instead of a class, since this div should be unique, but that's a tiny detail I admit it :) ). In the textile version however this isn't the case. I modified the two lines to change that and make the module more consistent: same behaviour regardless of input method used.

Have a nice day,
acp

CommentFileSizeAuthor
patch_160919 bytesacp

Comments

djorn’s picture

djorn commented

What I don't really understand is why the output formatting for the HTML and the Textile version of the footnotes are different. I basically edited footnotes.module to replace all _footnotes_replace_callback_textile references to _footnotes_replace_callback.

djorn’s picture

djorn commented

Oh I see now. This shows my ignorance. Sorry, wish I could delete the above comment.

hingo’s picture

hingo commented
Status: Needs review » Fixed

Indeed. This bug should now be fixed.

acp’s picture

acp commented

Hello,

thanks for the modification. I didn't replaced my footnotes.module yet, but after looking at the code of the current version, you didn't seem to apply the second correction, that is the

for the textile part of your module.

Is this by choice|accident, or did you correct it otherwise ?

hingo’s picture

hingo commented

Oh, that was on purpose, sorry for not commenting on that. In Textile mode, the module is not printing any html at all, so no divs or anything there. (I know it is allowed in Textile, but somehow I felt this is more correct?)

(About class/id, you are in fact right, on the other hand, no point to change it now.)

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)