Missing dark mode

Hi all!

Just to say that I’ve made a quick patch today that add the dark theme support to the default IPFire theme.

Here is how it looks:

  • Before patch

  • After patch

You can find more details here.

Please let me know what you think, I still have to add some CSS code for handling the tables correctly but it’s already usable.

I would be happy to have some guidance to make it more portable and persistent across updates if possible :slight_smile:

Thanks a lot for your work on IPFire, I really like it!

14 Likes

Very nice work. I hope it can get incorporated into a future release.

3 Likes

Hi @jiab77

Welcome to the IPFire community.

IPFire used to have several different themes but the problem that occurred was that any change to one of the WUI pages or the introduction of a new one required it to be checked with every theme and usually adjusted to allow things to work consistently with every theme, such as the next being visible in the chosen box etc.

This was starting to consume work for the core developers, and there aren’t many of them, and they were spending time trying to get themes working again due to a page cgi change rather than working on the core security and privacy aspects of IPFire.

So mid 2021 it was announced that the themes would be removed and there would only be the one theme left. It was said that if there was someone out there who wanted the themes and was willing to take on the responsibility for managing/maintaining them they were welcome to volunteer, they just needed to take it on as a long term activity, not just something for a couple of Core Updates.

There were a few complaints that the themes were going to be removed but no one came forward to become the maintainer of the themes.

In Core Update 156 the themes were removed. There were again a few complaints but no one came forward to act as maintainer.

If you want to have the option of a dark theme as part of the IPFire release you would be welcome to supply a patch to the IPFire source code to create it as long as you would be willing to maintain it and fix any bugs found with it, especially during the Testing phase of IPFire Releases.

If you are willing to do the above then a good first start is to join the IPFire devs mailing list.
https://wiki.ipfire.org/devel/contact
https://wiki.ipfire.org/devel/mailing-lists
You can then send a mail to that list to say what you are looking at doing and confirm your willingness to support it and ask for feedback. Keep in mind that they will want to be convinced that you are looking at supporting it long term.
Many addons were raised by people and submitted but have not been maintained with updates since then. This results in people like me doing updates to addons that I don’t use and have no ability to test so fingers are usually crossed that things will be tested by the addon users during the testing phase and any problems found but sometimes this does not occur until the actual release version.

You could also look at the how to build IPFire wiki pages so you can create a full ISO with any changes you are proposing and do a test install to confirm that everything works as expected.
https://wiki.ipfire.org/devel/ipfire-2-x/build-howto

See the following wiki link for how to create and submit patches.
https://wiki.ipfire.org/devel/submit-patches

Sorry for the rather long post but hopefully it will be helpful to you. If you are willing to join the IPFire developers you would be more than welcome and who knows, after a while you might be willing to get involved in other ways, such as solving bugs.

7 Likes

Thanks a lot for your nice comment :slight_smile:

2 Likes

Hi @bonnietwin

Thanks a lot for your very detailed reply! It’s exactly what I was looking for when asked for advice on how to make this patch more persistent across updates.

I know the IPFire project since the old times of SmoothWall and IPCop but always preferred the way IPFire did things. Unfortunately I always lacked time to contribute to the project.

I recently started to use IPFire again and then really needed something to lower the eye stress / pain created by the white backgrounds of the WUI. So yesterday I decided to make something to change that and finally make my eyes happy :grin:

It took me several hours to get the result you’ve seen but it still not finished, some parts like the tables are not managed yet, I’ll add the support today and post a message when available.

Now that you kinda know my motivations, I go back to your message and simply tell you that I totally understand your choices and the reasons why you had to drop the theme support on the WUI to be able to focus on what was really necessary and possible to do with your team size and resources.

I really want to have this dark mode patch included as part of the IPFire project and will follow the guidelines you gave me in your message.

However, to keep the dev process of the initial version easier, I’ll leave it as it is. Once everything has been fixed / managed correctly in that initial version, I’ll do all the necessary work to make it part of the IPFire source code and then drop the bash script currently used to install the patch or simply create a second version that will follow the IPFire build process and guidelines.

I don’t mind to maintain it as I really need it for my eyes and it’s also a great pleasure to help the IPFire community if I can with my work / contributions on the project.

Once started and more used to the IPFire build process, I might also push some patches to issues I had to fix on my own deployment. Some of them might also be useful for the community and not just for myself :sweat_smile:

Thanks a lot again for your nice welcome, It’s a great pleasure to have joined the IPFire community!

7 Likes

Just to say that I just did the upgrade to core 172 and the patch remained and didn’t broke anything in that new core upgrade :sweat_smile:

I ran a diff over the patched file and the only difference was the line added by install script:

[root@REDACTED ~]# cd /srv/web/ipfire/html/themes/ipfire/include/
[root@REDACTED include]# ll
total 28
-rw-r--r-- 1 root root  391 Feb 13  2020 colors.txt
drwxr-xr-x 2 root root   23 Mar 25  2022 css
-rw-r--r-- 1 root root 8040 Dec 29 06:33 functions.pl
-rw-r--r-- 1 root root 7993 Sep 13 13:00 functions.pl.before-patch
-rw-r--r-- 1 root root 8040 Dec 29 06:31 functions.pl.restore-patch
drwxr-xr-x 2 root root   32 Feb 13  2020 js
[root@REDACTED include]# diff -y --suppress-common-lines functions.pl.before-patch functions.pl
							      >
							      >		<script src="/include/darkmode.js"></script>
[root@REDACTED include]# diff -y --suppress-common-lines functions.pl.restore-patch functions.pl
[root@REDACTED include]# 

I can now focus on patching the tables to make them more readable when the dark mode is applied :grin:

I keep you updated.

2 Likes

Just a quick update, the process to better handle the pages rendered with tables has been started. To those who tested the first version, you can get the new one by doing the following:

# Remove the current version
cd ipfire-dark-theme
./patch.sh -r

# Get the latest version
git pull

# Install the latest version
./patch.sh

I’ve renamed the Todo section to Roadmap in the README to give a better view on what is planned on this project.

I wanted to see if the same work could be accomplished without Javascript and use CSS only but unfortunately it’s not possible as this feature is only supported by Firefox.

Properly handling the pages rendered mainly with tables required to write some path detection logic to apply the CSS code required by each related pages conditionally. I did my best and tested several different methods to avoid that but there was no other way without having to change the way the tables are rendered.

To those who are wondering if the code is leaking any data or does some “call-to-home”, the answer is simply no. I have no will nor interest into that. The code has been written to rely only a single part of the detected URL called path (or pathname in Javascript) and nothing else. The detected path is then only stored in the browser memory and nowhere else.

The goal of this project is to provide the missing dark mode without having to alter the WUI files to maximize the portability. However, one file needed to be patched in order to include to the Javascript file responsible of the CSS code injection, environment detection (just to detect if the user environment is configured in dark mode or light mode only) and store the selected mode (dark or light).

Doing so will require more work from my part but less work for the dev team which is something that I think they will appreciate :grin:

There is another goal which is to keep the visual guideline of the default IPFire theme and just add the missing “dark” touches where it is necessary. Unfortunately, to maximize the readability on darker backgrounds, I had to slightly changes some colors like the green, red and blue colors to lighter ones to reduce contrast balance issues.

@bonnietwin , I understand better now what you said about the difficulty to maintain several themes, just patching half of the pages with tables took me more than 10 hours and the work is not finished yet :sweat_smile:

I’m gonna sleep now but I’ll try to cover all pages with tables that needs to be handled better later today.

Sorry for that long and kinda technical post, I just wanted to give more details about how things are done and what the patch is exactly doing.

Hope you’ll like this new version :slight_smile:

3 Likes

@jiab77 You are doing a wonderful work. My eyesight will be forever thankful for your effort.

I need to say something though. As an IPFire user I care first and foremost about security, even above the WUI functionality.

Please, pretty please, please with a cherry on the top, make sure that this project will not bite the distro in the backside by introducing some future exploit of the WUI based on the use of javascript. That will be quite embarrassing for the project, even if there will be no other damage done, and if I remember correctly, there has been at list one exploit of the WUI in the past.

2 Likes

Hi @cfusco

Many thanks for your comment!

Don’t worry, I care a lot about privacy and security so all my projects are made with that in mind. Security is also part of my daily life so you can count on me to pay attention to that aspect.

Speaking about Javascript and hardening, as said in my previous post, I’ve invested several hours to find a way to detect the current path with CSS only using the @document at-rule but it is not supported by every browsers and instead, only supported by Firefox so unfortunately not a reliable solution.

Even if Javascript was already used in the initial version of the project, I wanted to see if everything could be rewritten in CSS only but it is not possible for the following reasons (at least):

  1. The lack of possibility to query the current path with CSS
  2. The way the tables are rendered from server side

As there is no way to target them properly from client side, I had to find a way to apply some CSS conditionally based on the detected current path and unfortunately, Javascript was required for doing that.

However, to reduce the attack surface involved with the use of Javascript, all the code has been written in vanilla JS and does not rely on any third party code like jQuery that is already included and used in the WUI (both rrdimage.js and refreshInetInfo.js are already using jQuery).

Even if the patch does not rely on any third party code, it can still be improved in term of security and it’s already added in the roadmap. To give you a little bit more details about that, I’ve planned to move the whole code into an IIFE (Immediately invoked function expression) to ensure that the code does not pollute the global scope.

To finish, I did a quick static analysis of the default Javascript code included in the WUI and spotted at least two things that could be improved:

  1. Move to the latest jQuery version (3.6.3) – Current version included: 3.6.0
  2. Move the current Javascript code in its own scope instead of the global one

I’ll stop here and go sleep :sweat_smile: but thanks a lot again for your nice comment and appreciation of my work but also for your care about security that we have in common.

2 Likes

Thanks for the heads up, I will put a patch together for that update and submit it.

Bear in mind that some of the WUI cgi pages are only shown if you are using certain options. So the Aliases page is only shown in the menu if the RED connection is Static and there are a few other similar ones. To test those pages the best option is to set up a vm testbed. I have done this with VirtualBox and have a virtual IPFire together with two virtual linux machines on Green, Blue and Orange so I can test various things out when a Test Release occurs or if I am fixing a bug and need to test the result.

The challenge of IPFire2.x is that the code is so intricately intertwined. This also occurs with the build to the extent that if an update to one package is required then the whole system has to be built.
IPFire3.x is being worked on as a from scratch rebuild of IPFire, which includes new capabilities such as IPv6 and more zones than just the max 4 of IPFire 2.x together with a build structure that means only a changed package and its dependencies will need to be rebuilt. Working on IPFire3.x is also where the core devs try and keep their focus on but any security or major improvement requirements on IPFire2.x take precedence for them.

I think it would be still good to join the dev mailing list and raise what you are doing while it is still a work in progress. The core devs have certain views on how things should be structured to fit in with how IPFire2.x is constructed and it would be good for you to take account of that now rather than have that raised when you submit a patch and it needs reworking.

I am not certain how the core devs would consider storing information in the browser, even if it is rather minimal. I believe their position on this would be to store any required status values in a file on IPFire itself. Either way, this might be a good topic to raise in the dev mailing list to get it clarified now rather than waiting till the patch is submitted.

Good luck with your ongoing work.

2 Likes

Hi @bonnietwin

Thanks a lot for your inputs. I’m sorry if few things said in my previous post has been taken as criticism, it has never been my intention. I know pretty much well how SmoothWall, IPCop and IPFire has been built so I can imagine how difficult it can be to make changes in them.

That’s why for this dark mode patch I think that releasing it as an addon would make the maintenance much easier and so not require to rebuild the whole system.

I’ve read a little bit about IPFire 3.x and I’m pretty much excited to see it coming! I don’t know if there is room where I can help but I’ll take a look. I would be happy to help if I can.

I’m not used to mailing lists at all but I’ll join it after this post. I’m more than happy to follow your guidelines and code writing rules, I have no will to lower the security provided by IPFire, I just wanted to make it more comfy for my eyes and if possible, make it useful for others.

As I said, I know how complex it can be to make a change in IPFire 2.x and really have no will to make it even more complicated, that’s why I opted for adapting my own code to the current structure instead of asking you guys to change things for me.

I understand your point about having user data even minimalist stored on the client side. I myself prefer storing sensible data always on server side rather than the client side for obvious security reasons.

I’ll discuss about that on the dev mailing list but to quickly explain my choices / thinking, I didn’t wanted to use the invasive cookies for that purpose, so I usually opt for sessionStorage feature instead of localStorage as the stored data are not persistent and vanish at the moment you close the tab or the browser window which is not the case with localStorage.

I just told myself that asking the user that does not have his system properly detected as already configured for dark mode to always select the dark theme on each reload would have been pretty annoying, so that information needed to be stored somewhere.

But you’re right, if previously discussed with the IPFire devs, I can make the code reading the theme preference from a server side value instead of a client side, that would be perfect!

To finish, regarding jquery, I just spotted it during the code analysis I did to understand / find the way to apply my changes from client side without having to change things on server side. I had no will to finger point or criticize what has been done. It’s always easy to criticize but much harder to come with a solution along the critics. So I’ll join the dev mailing and see if maybe there is a will to drop jquery completely from the WUI and rewrite the small JS code portions without it. I would be happy to help on that too.

Thanks a lot for your advice and sorry if I did things incorrectly.

3 Likes

@bonnietwin just wanted to add that you don’t have to rush for updating jquery as I’ve checked last night and there is no security issues that has been patched, the newer versions just contains fixes for different logic and parsing bugs.

1 Like

Hi @jiab77,

First of all, thank you for bringing a dark mode to the WUI! This is something I’ve wanted to do for a long time but unfortunately don’t have the time to work on properly.

Would this also affect pakfire.js? I submitted these patches last year, in an effort to make the WUI more responsive and user friendly. As far as I remember, nobody objected to the use of jQuery.
Is there an issue with the code that I should work on? Feel free to raise a bug in bugzilla.ipfire.org!

1 Like

Don’t worry I did not take anything as criticism. I just want to help you increase the chances of getting your patch accepted by the core devs.

Unfortunately, the way the IPFire2.x build system is constructed, it makes no difference if it is an addon or a core change the build system still needs to be fully run.

Again, don’t worry about this. I was glad that you flagged it up.
I don’t have the knowledge or skills related to doing development of the core functions of IPFire. However, I am able to support the core devs by doing patch updates for the packages.
There are more than 650 separate external packages that go to make up IPFire or its addons. I try and find any updates of the packages so I can submit an update patch. However, after more than a year doing this, I still managed to find an addon package recently that was last updated in 2005.
Therefore I was very happy that you flagged up that jquery had a new version, so I could deal with it :+1:

No you haven’t done anything incorrectly and my apolgies if it came across like that. It was not my intention.
A good starting point is always in the forums. Then one of the moderators can suggest that what is being worked on should be taken over to the dev mailing list. The core devs tend to be very busy and are only able to occasionally have a look at what is happening in the forum so may miss things.

So how you have raised this up is absolutely the correct way.

3 Likes

Hi @luani

I’m really glad you liked my work and it was for such reasons that I made it. Not be famous but to provide something that could be useful for others and was missing in the IPFire WUI.

Regarding jQuery, I have absolutely nothing against it, I was just replying to @cfusco who was asking me to pay attention to the security aspect in the project and in this regard, I just said that removing jQuery if not completely necessary would also increase the WUI security in the way that the less you have third party code to manage the better and easier it is to maintain it.

jQuery is very useful and has a lot of features but if not maintained, can also expose the WUI at risks and increase the attack surface.

For the dark mode patch, I didn’t wanted to rely on it and instead I’ve tried to make the code free from any third party dependencies in the hope to make it as much portable as possible. Even the two icons used in the project has been taken from the Icon8 website and downloaded in base64 so that there is no external requests made from the internal network to load them.

The patch itself, just inject the CSS code defined in patch.js that is then renamed to darkmode.js when installed to make it more clear about what it does exactly. Doing so avoids the need to load an external CSS file.

I really tried to make it safe to use but got the feeling that I haven’t done things correctly so I’ll try my best to make things better.

Regarding pakfire.js, I haven’t checked it’s code so I can’t tell about it but again, I have really nothing against jQuery and if nobody objected about it, then it’s totally fine :grin:

I just wanted to say that if you are using jQuery only for targeting DOM elements then it can quickly be replaced by a simple wrapper around document.querySelector and document.querySelectorAll. But if you are using jQuery for more complex things then it worth to be kept :wink:

As I might have found a bug in one table rendered in update accelerator cgi, I’ll raise a bug in bugzilla. I’m just waiting to receive the confirmation / approval of my subscription to the developer mailing list before raising the bug.

Thanks a lot again for your appreciation of my work, most of my projects don’t really get interests from others so it’s nice to see that this one is nicely welcomed by the community :bowing_man:

4 Likes

Hi @bonnietwin

Thanks a lot for your reply. I’m glad to not have offended anyone in the team as it was absolutely not my intention. I have huge respect for the all the team that make IPFire still alive after all these years! I just wanted to contribute to it the way I could and it was on the WUI side. If there is other rooms where I can help, I’d be pleased to do it!

Unfortunately, the way the IPFire2.x build system is constructed, it makes no difference if it is an addon or a core change the build system still needs to be fully run.

I didn’t knew about that so thanks a lot for making it more clear to me. I still think that packaging the patch as an addon would make it more easier for users to install and remove it but I might be wrong on that so I’m waiting for the approval of my subscription to the developer mailing list to get a better view on that.

I don’t want to increase the workload that the team already have and just want to make things easier instead and handle part of the work if not the whole on my side with pleasure.

There are more than 650 separate external packages that go to make up IPFire or its addons.

Damn! That’s a lot! Much respect for managing all of these! :muscle:

I still managed to find an addon package recently that was last updated in 2005.

Totally understandable and you flagged it so it’s the most matter :wink:

Therefore I was very happy that you flagged up that jquery had a new version, so I could deal with it :+1:

I’m glad if I could have helped you, that’s all what I wanted, to be helpful.

No you haven’t done anything incorrectly and my apolgies if it came across like that. It was not my intention.

No worries, I’m sorry too for having taken things in the wrong way :bowing_man:

So how you have raised this up is absolutely the correct way.

Awesome! But next time, I’ll simply fill a bug report. It was fine for this one because there is no security issues behind but it would have been a bad way if that was for disclosing a serious vulnerability :sweat_smile:

Anyway, it’s a great pleasure to contribute to the IPFire project and also see that people appreciate my small contribution to this great project.

3 Likes

Agreed! I think zoneconf.js is a good example, it only really needs -and uses- a simple document.getElementById to do it’s job.
But rrdimage.js uses jQuery data attributes and pakfire.js relies on JSON/Ajax. It’s certainly possible to do that in vanilla JS, but it would require a lot of work, testing and ongoing maintenance to get it right.
So I’d vote to keep jQuery (in place and updated) :slight_smile:

Have you confirmed this with their licensing policy? I believe icons8 requires you to provide a visible link to their library. At least for most of their icons.

P.S. I like how you used simple CSS to invert the graphs! I contemplated about switching color palletes in graphs.pl, but that would be so much work.

4 Likes

Yeah exactly, if jQuery is heavily used, no need to rewrite everything if it works perfectly :grin: just keeping it updated is largely enough.

Have you confirmed this with their licensing policy? I believe icons8 requires you to provide a visible link to their library. At least for most of their icons.

I did and mentioned their name in the code but you’re right, it might not be enough and to avoid any licensing problems, I’ll replace them with open source ones from feather.

P.S. I like how you used simple CSS to invert the graphs! I contemplated about switching color palletes in graphs.pl, but that would be so much work.

Thanks a lot for your nice comment, I use the same trick to force websites that don’t offer a dark theme to become darker locally using a simple bookmarlet. In it, there is filter: invert(100%) injected in the page along with other CSS properties and this gave me the idea to reuse the invert filter to invert the colors of the generated rrd graphs without having to modify something on server side.

I liked the result and hoped that others would like it too :sweat_smile: Apparently it’s the case so I’m pretty happy about that :slight_smile:

2 Likes

I just finished the work to replace them by open source ones from Feather. You can check the commit here.

1 Like

If you want, I can also vary the inverted colors by adding hue-rotate(Ndeg) as second filter. There is this excellent article from Wikipedia if you wanna know more about it. Just let me know as maybe the defined colors for rrd graph have specific meanings which are now misleading due to the fact that colors gets inverted. Using the hue-rotate filter might help to select better colors.

3 Likes