| Summary: | Core Update 172: Update accelerator page | statistics - Table style rendering bug | ||
|---|---|---|---|
| Product: | IPFire | Reporter: | Doctor Who <jiab77> |
| Component: | --- | Assignee: | Adolf Belka <adolf.belka> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | Aesthetic Issue | ||
| Priority: | - Unknown - | CC: | adolf.belka, matthias.fischer, peter.mueller |
| Version: | 2 | ||
| Hardware: | unspecified | ||
| OS: | Unspecified | ||
| Attachments: | Diff for the bug fix | ||
Good catch... ;-) I don't use the update accelerator, so I never ran into this bug. If there is no other way to get this working - one thing you could test: Add these line to '/var/ipfire/header.pl' at line ~33: ... $Header::table3colour = '#D6D6D6'; # for color20 ... Actually, "table2colour" uses the same values as "color22" - I don't know why the assigned color should differ... Then change your working string accordingly to: print "<tr bgcolor='$Header::table3colour'>\n"; } Does this help? jm2c (In reply to Matthias Fischer from comment #1) > Good catch... ;-) > > I don't use the update accelerator, so I never ran into this bug. > > If there is no other way to get this working - one thing you could test: > > Add these line to '/var/ipfire/header.pl' at line ~33: > > ... > $Header::table3colour = '#D6D6D6'; # for color20 > ... > > Actually, "table2colour" uses the same values as "color22" - I don't know > why the assigned color should differ... > > Then change your working string accordingly to: > > print "<tr bgcolor='$Header::table3colour'>\n"; } > > Does this help? > > jm2c Hi Matthias, Thanks a lot for your reply! To be honest, I just been lucky to find it as I was simply looking at how to implement the dark mode patch for the tables and found this bug so it's pretty much by accident that I found it :D I'll do the suggested changes and let you know if it worked or not. I'm just wondering, as I'm not part of the dev team, how the patch will be implemented in the project? ++ Jiab77 I just did the test with the suggested changes and it worked perfectly so I've added required changes in my dark mode patch: https://github.com/Jiab77/ipfire-dark-theme/commit/48abd705db3f91bf1bac1fd5a96db468efef6928 Here is the resulting code: * /var/ipfire/header.pl [root@REDACTED ipfire]# grep -n "::table3" -C1 header.pl 32-$Header::table2colour = '#F0F0F0'; 33:$Header::table3colour = '#D6D6D6'; 34-$Header::colourred = '#993333'; * /srv/web/ipfire/cgi-bin/updatexlrator.cgi [root@REDACTED cgi-bin]# grep -n "::table3" -C1 updatexlrator.cgi 829- # print "<tr bgcolor='$Header::table1colour'>\n"; } # WORKS / FIX BUG 830: print "<tr bgcolor='$Header::table3colour'>\n"; } # WORKS / SUGGESTED BUG FIX 831- else { And the resulting code seen from client side: <tr bgcolor="#D6D6D6"> That I can target with my patch using this selector: .bigbox table:nth-of-type(4) tr[bgcolor="#D6D6D6"] td.base And alter the text color depending if light or dark theme is applied or not. For me your suggested fix did the job but I have no idea how this can now be added to the main project source. Let me know if I can help on something else. ++ Jiab77 Hi Guys! Any news regarding this bug report? :) ++ Jiab77 bgcolor was deprecated in the W3C HTML 4.0 Specification and in some cases and with some browsers the command no longer works. This has resulted in some bgcolor commands not showing any colours in the dhcp.cgi page. I have been working on replacing bgcolor with CSS following an approach done by @Leo with another page. Once the dhcp.cgi patch is submitted I will follow the same approach for updatexlrator.cgi While some bgcolor lines are still working, I think it is best to replace all of them with CSS based approach otherwise at some later date more of the bgcolor lines will stop working. (In reply to Adolf Belka from comment #5) > bgcolor was deprecated in the W3C HTML 4.0 Specification and in some cases > and with some browsers the command no longer works. > > This has resulted in some bgcolor commands not showing any colours in the > dhcp.cgi page. I have been working on replacing bgcolor with CSS following > an approach done by @Leo with another page. > > Once the dhcp.cgi patch is submitted I will follow the same approach for > updatexlrator.cgi > > While some bgcolor lines are still working, I think it is best to replace > all of them with CSS based approach otherwise at some later date more of the > bgcolor lines will stop working. I'm totally agree with you if I may say something about that. As step by step browsers are removing legacy code from their codebase, at some point these old HTML properties will not be supported at all and so moving them to CSS will definitely avoid this issue to hit the project in the future. Created attachment 1137 [details]
Diff for the bug fix
I have created a fix for this using css approach as used for the zoneconf.cgi and dhcp.cgi (patch recently submitted)
I can tell that the percentage bar is working correctly as I can see that when I press the Statistics button. However I don't see any table as I don't use it for my update downloads normally as I use Arch Linux for all my systems and the downloads for updates are done via https.
I tried downloading some linux update files from some Ubuntu http sites but still I could not get updatexlrator.cgi to notice them and put them in the cache.
@DoctorWho can you either tell me where to download some files from so that they get recognised by updatexlrator, including any modifications required in the conf file, or can you please test the attached diff to confirm that it does work and provides the tables correctly formatted.
patch to fix this bug has been submitted to the dev mailing list and to patchwork. https://lists.ipfire.org/pipermail/development/2023-February/015493.html https://patchwork.ipfire.org/project/ipfire/patch/20230221131154.4579-1-adolf.belka@ipfire.org/ (In reply to Adolf Belka from comment #7) > Created attachment 1137 [details] > Diff for the bug fix > > I have created a fix for this using css approach as used for the > zoneconf.cgi and dhcp.cgi (patch recently submitted) > > I can tell that the percentage bar is working correctly as I can see that > when I press the Statistics button. However I don't see any table as I don't > use it for my update downloads normally as I use Arch Linux for all my > systems and the downloads for updates are done via https. > > I tried downloading some linux update files from some Ubuntu http sites but > still I could not get updatexlrator.cgi to notice them and put them in the > cache. > > @DoctorWho can you either tell me where to download some files from so that > they get recognised by updatexlrator, including any modifications required > in the conf file, or can you please test the attached diff to confirm that > it does work and provides the tables correctly formatted. Hi Adolf, Sorry for my late reply. I had the same issue when configuring the updatexlrator on my test VM. In fact you need to allocate / define a large enough amount of available storage space to make it work. Got the feeling that the updatexlrator needs at least 50% of the storage space available to allow it to store the update packages in its cache. Regarding your questions, I've seen updatexlrator correctly caching Apple and Linux update files (Ubuntu on my side for Linux) And regarding the storage space required to make it work, I had to set it to 75% as "max disk usage" Hope this will help you a little bit. PS: If there is files I can download somewhere and test them on my test VM, just let me know and I'll try my best to make the tests ASAP. (In reply to Adolf Belka from comment #8) > patch to fix this bug has been submitted to the dev mailing list and to > patchwork. > > > https://lists.ipfire.org/pipermail/development/2023-February/015493.html > https://patchwork.ipfire.org/project/ipfire/patch/20230221131154.4579-1- > adolf.belka@ipfire.org/ Hi Adolf, I've checked your code in the links and it sounds good to me visually. I'll test it on my VM and let you know how it goes. ++ Jiab77 Fix has been merged into next (future CU174) https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=7721b92500e2e060944f5fe92b956b4ce1caea51 I have tested this in my vm testbed system with CU174 Testing. The percentage box looks correct but I can't test the statistics table as I don't normally use the Updatexlrator so have no data to be shown in the table. It would be good if the original bug reporter @DoctorWho could test CU174 Testing out and confirm that the problem has been resolved. |
Overview: There is a minor table style rendering bug in the statistics section of the update accelerator page. Steps to Reproduce: 1. Go to the update accelerator page, 2. click on the statistics button, 3. look at the "Statistics by source" table, 4. right click on the first line 5. and select "Inspect" in the contextual menu. It will open the web console and display the HTML source code of the corresponding line. Actual Results: The parent <tr> element which construct the table line seems to have a quoting issue and has been rendered incorrectly: "<tr bgcolor="" #d6d6d6'="">" Expected Results: The parent <tr> element should be rewritten from: <tr bgcolor="" #d6d6d6'=""> To: <tr bgcolor="#d6d6d6"> In order to display the defined color properly along the line (table cells). Additional Information: The bug has been spotted in IPFire Core Update 172 using Chromium version 108.0.5359.71 (latest one for Ubuntu 20.04) Initial debug: From what I can see on server side, the bug is located in lines 826 and 828 of the updatexlrator.cgi file [root@REDACTED cgi-bin]# grep -n bgcolor --color=always updatexlrator.cgi 535: print "<tr bgcolor='$Header::table1colour'>\n"; } 537: print "<tr bgcolor='$Header::table2colour'>\n"; } 826: print "<tr bgcolor=''$color{'color20'}'>\n"; } 828: print "<tr bgcolor=''$color{'color22'}'>\n"; } 1165: print "<tr bgcolor='$Header::table1colour'>\n"; } 1167: print "<tr bgcolor='$Header::table2colour'>\n"; } Additional debug: I've tried to fix the bug on my own but my knwoledge in Perl are quite limited :D However I could find a potential fix: 826: # print "<tr bgcolor=''$color{'color20'}'>\n"; } # BUG 827: # print "<tr bgcolor='$color{"color20"}'>\n"; } # BOOM / ERROR 500 828: # print "<tr bgcolor='$color::color20'>\n"; } # NOT WORKING 829: print "<tr bgcolor='$Header::table1colour'>\n"; } # WORKS / FIX BUG 831: # print "<tr bgcolor=''$color{'color22'}'>\n"; } # BUG 832: # print "<tr bgcolor='$color{"color22"}'>\n"; } # BOOM / ERROR 500 833: # print "<tr bgcolor='$color::color22'>\n"; } # NOT WORKING 834: print "<tr bgcolor='$Header::table2colour'>\n"; } # WORKS / FIX BUG Suggested fix: Simply replace these two lines: 826: print "<tr bgcolor=''$color{'color20'}'>\n"; } 828: print "<tr bgcolor=''$color{'color22'}'>\n"; } By the same code used in lines 535, 537, 1165 and 1167. It should solve the table style rendering bug. I did the tests on my side and got the expected HTML output: <tr bgcolor="#E0E0E0"> But the assigned colors are different from those called with "color{'color20'}" and "color{'color22'}" so I don't know if that is the result you would expect on your side. Hope to have done things correctly and saved you some precious time in debugging this issue. ++ Doctor Who (Jiab77)