Render links in article comments

[RoukePouw]

All links in comments will be formatted:

www.abc.org -> <a href="www.abc.org" target="_blank">www.abc.org</a>

[RoukePouw]

I tried handling the dots at the end of a link but encountered some curious behavior where the dot suffix would only appear if I add a string before the anchor...bit mind-boggled at the moment why this would matter.

return 'A<a href="' . $url . '" target="_blank">' . $url . '</a>' . $suffix; // curiously this does show the suffix

return '<a href="' . $url . '" target="_blank">' . $url . '</a>' . $suffix; // this does not show the suffix

[matthijskooijman]

I'm seeing the same thing here indeed. Seems like the $this->html->create() eats text outside of nodes if it contains <a> tags (but not when it has <p> or <br/>). Maybe because create() isn't perfectly made for this or something? Anyway, a practical fix seems to be to just put the comment text in a <p> (might need some CSS changes, haven't checked that). Could you try that?

All this HTML could maybe be restructured (at least some classes added), but that's out of scope for now.

[RoukePouw]

Thanks! I wrapped it in a span to prevent layout changes.

[matthijskooijman]

@RoukePouw, seems there still some issues with comment rendering. On the live server, we noticed that some comments (seemingly long comments) are not displayed at all. For example, this comment:

Reacties worden, voor zover ik weet alleen verwijderd als deze onder pseudoniem of in ieder geval niet duidelijk onder eigen naam zijn ingestuurd. Ze zijn ook niet welkom als er te veel op de man of vrouw wordt gespeeld. Reacties zijn – denk ik – wel heel welkom als deze een bijdrage leveren – hoe dan ook - aan de discussie over het onderwerp. Wat de Wagenwerkplaats Oost betreft heb ik kennis van zaken. Vanaf het begin van de discussie over de toekomst van het verlaten gebied van de Wagenwerkplaats in haar geheel heb ik daaraan een bijdrage proberen te leveren. Ik heb de ontwikkeling van het gebied gevolgd sinds 2003. Het eerste streven - vanuit het Soesterkwartier gezien - was, om het erfgoed van de Wagenwerkplaats te redden. Mede dankzij de inzet van de Stichting Industrieel Erfgoed Amersfoort (tegenwoordig afgekort als SIESTA) zijn een aantal historische gebouwen en de twee buitenrolbanen, en een exact omschreven direct omliggend gebied als industriëel ensemble op de Rijksmonumentenlijst geplaatst. (2007). Die actie tot behoud vanuit het Soesterkwartier zelf is destijds vooral ingegeven door de vrees, dat onze mooie historisch gegroeide wijk afgesloten zou worden van het spoorwegemplacement, door een aaneengesloten afsluitende reeks zeer hoge kantoorgebouwen. Er is indertijd al heel goed door velen aangevoeld dat dit geen goede bijdrage aan onze wijk zou betekenen. Het Soesterkwartier is er altijd voorstander van geweest dat de Wagenwerkplaats tot het Soesterkwartier gerekend zou moeten worden, evenals dat het gehele gebied ten noorden en vanaf de Groengordel tot aan de Eem, historisch gezien tot het Soesterkwartier gerekend kan worden. Zelfs de postcode 3812 herinnert ons daar nog aan. Samen 3812. De Amsterdamseweg die nu als een barrière dit historische Soesterkwartiergebied ten westen van het stadscentrum doorsnijdt is pas in de jaren ’50 aangelegd. De pogingen om het gehele gebied vanaf het spoor tot aan de Eem weer te herenigen tot een geheel nieuw Stadsdeel en als een parel in de armen van omze stad te sluiten, juich ik toe. Het zal een proces zijn van vele tientallen jaren. Over de wijze waarop is veel discussie, en daar is nog wel veel goede samenwerking voor nodig. Ik geloof nog steeds dat wij daartoe samen – misschien na wat moeizaam overleg – wel uit kunnen komen. Veel hoogbouw vlak bij de Piet Mondriaanlaan wijs ik echter af aangezien het dan in dat gebied één samenklontering van steen zou gaan worden. Geen Manhattan aan de Eem, maar zeker ook niet vlak bij de Piet Mondriaanlaan. Eigenlijk heeft de architect van het gebouw Eempolis, indertijd te weinig naar de te verwachten ontwikkelingen in de toekomst gekeken. Met het ontwerp van de rechtervleugel van dit gebouw heeft hij de Wagenwerkplaats de vorstelijke entree ontzegd, die dit prachtige gebied eigenlijk wel verdient. Als je op die hoogte dan ook te veel hoogbouw plaatst dan sluit je de rest van de Wagenwerkplaats onnodig af. Laten we eerst daarom nog wat meer en constructief met elkaar discussiëren. 

is not shown. Investigating shows that preg_replace_callback() returns null and preg_last_error() returns 2 (which I think is PREG_BACKTRACK_LIMIT_ERROR). So it seems like the regex might be inefficient in a way where it relies too much on backtracking, which breaks for long comments? Not sure if we want to manually debug this regex, though... Any thoughts?

It does not seem to be just long comments, since I tried 5 paragrpahs of lorem ipsum (with a dozen of leading newlines accidentally it seems) and found that somehow that wasn't displayed either, but there the preg didn't return an error. A little more testing shows that the problem is likely that the comment started with a newline, which probably triggers the same problem with create stripping text content. That probably means we need to put the entire comment body into a span or div, rather than just the <a> tags after all, since the same problem seems to happen for <br> too...

[RoukePouw]

I've replaced the regex with a simpler one and added a check to catch if the regex fails

[matthijskooijman]

Thanks!

Simpler looks good, though I now wonder if it's not too simple now. In particular, I think this regex matches any two words separated by a dot (but not spaces), e.g. foo.bar. Not highly likely to be triggered in practice, but maybe just a little too likely anyway. I wonder if we should match explicit https?:// links, or domain names starting with just www.? Then you'd miss a lot of other domains, but that's ok, I think.

Also, I wonder if, now the regex is simpler, we could just include the "exclude the trailing dot"-stuff in the regex now? Basically, you can just duplicate the list [...]+ block and remove the . and the + to force not ending in a space (though it might end up needing to backtrack and cause problems again)?

[RoukePouw]

I wonder if we should match explicit https?:// links, or domain names starting with just www.? Then you'd miss a lot of other domains, but that's ok, I think.

Agree :)

Basically, you can just duplicate the list [...]+ block and remove the . and the + to force not ending in a space (though it might end up needing to backtrack and cause problems again)?

I followed your suggestion. That seems to do the job. But it's $this->html->create that seems to do something weird. (again?)

const URL_REGEX='/(https?:\/\/)[\w\/-]+\.[\w\/-?=%.&#]*?[\w\/-?=%&#]/i';
// [protocol://] [word] . [suffix /path ?query&parameter=value#bookmark  ] [any url char but a dot ]

input:

http://bsdf.sdfsdf?4354&454. 
http://bsdf.sdfsdf?4354&454. a 
http://bsdf.sdfsdf?4354&454. 
http://bsdf.sdfsdf?4354&454. v

the generated $commentHtml:

<span>
    <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
</span>
.
<br/>
<span>
    <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
</span>
. a
<br/>
<span>
    <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
</span>
.
<br/>
<span>
    <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
</span>
. v 
<p>
    door 
    <strong>test</strong>
     om 20-12-20, 07:03
</p>
<br/>

The resulting html source.

<span>
     <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
 </span>
 <br>
 <span>
     <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
 </span>
 <br>
 <span>
     <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
 </span>
 <br>
 <span>
     <a href="http://bsdf.sdfsdf?4354&amp;454" target="_blank">http://bsdf.sdfsdf?4354&amp;454</a>
 </span>
 <p>
     door 
     <strong>test</strong>
      om 20-12-20, 07:03
 </p>

It seems that $this->html->create($commentHtml); does something unexpected that causes some postfix strings (dots and characters) to disappear?

[matthijskooijman]

@RoukePouw I put your commits with some additions in https://github.com/PlanBCode/hypha/pull/364. I think that addresses all open questions, but let's continue discussion there.