tematres / TemaTres-Vocabulary-Server

Web application for management formal representations of knowledge, like controlled vocabularies, taxonomies, thesauri and glossaries
https://vocabularyserver.com
118 stars 52 forks source link

Term hyperlinks are styled differently depending on PHP version #84

Closed a-roussos closed 12 months ago

a-roussos commented 1 year ago
Click here to read some background information (aka the TL;DR section!) We have a local TemaTres installation accessible only from hosts in our Intranet, which we use as an ever-growing thesaurus of theological terms (we call it "ReligTres" 😲). This is also where development takes place, as we have customised parts of the code to suit our needs (for example the addition of a modal login form, which eventually found its way upstream in #44 thanks to Diego/@tematres 😉). A dump of the local TemaTres database is uploaded daily to a public-facing instance of the same thesaurus which you can see [here](https://library.imparaklitou.gr/religtres). The local source code tree is also synced (though less frequently) with that of the remote server (usually after any changes have been tested and found to be working OK). Earlier this week I upgraded the O/S that hosts our local TemaTres instance from Debian 11.7.0 to 12.1.0. This involved an update of PHP as well (now running version 8.2.7). I also upgraded our local TemaTres from [v3.4](https://github.com/tematres/TemaTres-Vocabulary-Server/releases/tag/v3.4) to [v3.4.3](https://github.com/tematres/TemaTres-Vocabulary-Server/releases/tag/v3.4.3). After this, we noticed that the ["About.."](https://github.com/tematres/TemaTres-Vocabulary-Server/blob/master/vocab/sobre.php) page was broken, with a PHP Fatal error being written in the Apache log files. To fix this, I cherry-picked all remaining commits to master since v3.4.3 (on top of our local Git branch) (991dec2 being the crucial commit) -- the "About..." page issue was gone. So, basically, we were happily running the bleeding edge of the source code (plus any local changes we've made, of course). Since no other major issue was observed in our testing, I rsync'd the local source tree with the remote server.

And now, for the actual issue 😛:

We noticed that the hyperlinks for Accepted Terms (estado_id = 13) shown in the "Recent changes" page (vocab/index.php?s=n) are styled differently (green colour vs. blue) between two TemaTres installations that we administer. Both installations are running off the same source code tree (essentially the upstream TemaTres master plus some changes we've made), but are on different servers. Server 1 is a Debian 12.1.0 system with PHP 8.2.7. Server 2 is a Debian 10.13.0 system with PHP 7.3.31 -- this is where the issue was observed. In server 1, the hyperlinks are green (correct), but in server 2 the links are blue (incorrect).

Here's a couple of screenshots to illustrate what's happening (click to expand this section): ![screenshotOK](https://github.com/tematres/TemaTres-Vocabulary-Server/assets/25583206/25d59c3a-e144-4b3c-ba40-3c3ff3ec5691) ![screenshotBAD](https://github.com/tematres/TemaTres-Vocabulary-Server/assets/25583206/e84d0225-4cbe-4d67-a691-dfefeeb01817)

As you can see above, in server 2 the estado_termino13 class is missing from the a elements. Therefore, the styling applied to the hyperlinks (CSS color property) comes from the default Bootstrap v3.3.7 theme instead of being overriden by TemaTres's own CSS.

The commit that introduced the green styling is 190c826.

The HTMLlistaTerminosFecha function is responsible for building the table of Terms in the "Recent changes" page. Inside the while loop, HTMLlinkTerm is called for each Term fetched from the database with SQLlastTerms:

https://github.com/tematres/TemaTres-Vocabulary-Server/blob/190c8265644225774df34e74ba2ab9ba5ae426ad/common/include/fun.html.php#L1193-L1232

https://github.com/tematres/TemaTres-Vocabulary-Server/blob/190c8265644225774df34e74ba2ab9ba5ae426ad/common/include/fun.html.php#L1941-L1961

https://github.com/tematres/TemaTres-Vocabulary-Server/blob/190c8265644225774df34e74ba2ab9ba5ae426ad/common/include/fun.gral.php#L1770-L1775

The important change from 190c826 is this:

@@ -1884,7 +1942,8 @@ function HTMLlinkTerm($arrayTerm, $arg = array())
 {

-    $class=(@$arg["style"]) ? $arg["style"] : '';
+    $class=(strlen(array2value("style", $arg)>0)) ? $arg["style"] : '' ;
+
+    $class.=(array2value("isMetaTerm",$arrayTerm)==1) ? ' metaTerm' : '' ;

-    $class.=($arrayTerm["isMetaTerm"]==1) ? ' metaTerm' : '';

     $url_parts=parse_url($_SESSION["CFGURL"]);

The culprit is this line:

$class=(strlen(array2value("style", $arg)>0)) ? $arg["style"] : '' ;

Notice how the argument to strlen is array2value("style", $arg)>0 but I'm guessing that @tematres meant to write this as:

$class=(strlen(array2value("style", $arg))>0) ? $arg["style"] : '' ;

In fact, if I move the closing parenthesis for the strlen call two characters to the left, I get green hyperlinks on both servers!

In case anyone's interested, I've set up a small code sample here to investigate why we get different behaviour based on the PHP version.

tematres commented 12 months ago

Hi @a-roussos !! Thanks for the review and the bug fix! :) cheers!