Missing dark mode

I restarted the device and it came back. It was an good idea to reinstall it, because now the button to turn on/off is working. I couldn’t see any changes after the update only.

The wireless client + captive portal tables are still hard to read:

1 Like

Hi all,
@jiab77 thanks for your nice work and your effort in here :+1: .
A small enhancement of missing pages, in ovpnmain.cgi the ‘Static IP address pools’

lacks also the respective table color :blush: .

Thanks again. All the best,

Erik

EDIT: Version is

 - New patch version: 0.3.10
 - New installer version: 0.3.24

Removing dark mode patch... done

Installing dark mode patch... done
``
1 Like

obraz


Best

1 Like

Ok great! Your reported issue has been fixed btw except the one related to the graph from the WIO add-on. The issue on this one is due to the fact that the graph is not on the main page context but in an iframe which means my patch can’t reach this context.

I’ll have to write some iframe context hack to reach them and I’m not sure everyone will agree with that as it would mean breaking all boundaries that browsers are enforcing to patch the graphs from the WIO add-on only.

Guys, can you tell me what you think about that?

1 Like

Hi @ummeegge,

Thanks a lot for your reported issue! I’ve fixed it in the latest release and also included your name in the README as contributor.

Please run ./patch.sh -u from the project folder and you should get the latest release.

1 Like

Hi @xperimental,

I’m glad you could make my patch working again! I’ve fixed your reported issues so if you run ./patch.sh -u from the project folder, you should see the fixes applied.

Hi @sec-con,

I’ve improved the CSS selectors so I hope that your issue on the DHCP page is now fixed.

Regarding the issue you had with the patch.sh script, I’m planning to add a reset feature that will restore the original functions.pl file, remove all created backup files and reinstall the patch. I hope that will help when the new release does not get installed correctly for unknown reasons :sweat_smile:

Another thing, I appreciate very much you take the time to write the cli input needed for each step. I understand that CLI is handy in many situations and use it if absolutely necessary, but overall I just avoid it. With your guidance I am actually accomplishing something, and also learning a bit in the process.

I know that using the terminal can be something very frightening for some persons so I’m always trying to make the commands easy to understand and also easy to run if possible so I’m glad that it has helped you :grin:

1 Like

Hi guys,

Sorry for my silence, it took me some time to debug and fix all the reported issues and also worked on some other unrelated projects.

As you might understood, I’ve made a new release and both patch and install script are now version 0.4.0. Here are the main changes:

  • You can now see the loaded patch version in the web console
  • Most of the reported readability issues has been fixed
  • Strict type check has been enforced
  • Reduced global scope pollution
  • debugMode boolean setting is now user reachable

For those who are comfortable with the web console, you can enable/disable the debug mode that way:

  1. Open the web console
  2. Enter:
debugMode = true;
  1. Enable / disable the patch with the icon selector

You should see more debugging outputs in the console.

If you want to revert your changes, you can simply reload the page or just set it back to false:

debugMode = false;

That’s all for this latest version. I’ll now focus on the rest of the planned tasks in the roadmap.

4 Likes

can the WIO addon be patched not to use an iframe? I mean, not necessarily by you. You could just open a bug report asking for the change and leave your dark mode unchanged in this regard.

2 Likes

As the original owner for that addon has stepped back from it after many, many years supporting it, I have picked that package up to maintain it.

I would have to figure out what iframes are and what alternative should be used.

To warrant changes like that that my view would be that the dark mode would need to be submitted and accepted as a formal addon in IPFire rather than a third party addon that is not reviewed within the IPFire environment but requires IPFire to be changed to fit in with it.

2 Likes

Hmm, iframes are not very safe. There are alternate implementations that might make them safer like using sandbox Securely embed content on your site - Chrome Developers

Having said that, one could perhaps use Ajax or embed, but I have yet to find out how to implement that - looking for something along those lines for a completely different thing.

Found something related to Ajax
http://www.javascriptkit.com/dhtmltutors/ajaxincludes.shtml

Just for information
Below are links to the found pages.
I hope they will be helpful.

Regards

2 Likes

Hi all,

just did it and all looks good, thanks again.

Best,

Erik

2 Likes

Hi Adolf, this reminds me of my work on graphs.pl and getrrdimage.cgi: git.ipfire.org Git - ipfire-2.x.git/commit

I would like to suggest that you include the WIO graphs in the getrrdimage.cgi sytem. That way, you can remove some duplicate code and also use the automatic graph refresh JS.

2 Likes

Hi Leo, that sounds like a good idea, separate from any dark mode requirements, because it goes towards standardisation.

I will have a look at your approach and come back to you if i have any questions.

EDIT:
Have had a look through your changes in graph.pl compared to the wio-graphs.pl and I can see the similarities there. That gives me confidence that I should be able to look at adding the wio graphs into the getrrdimage.cgi system using the @luani approach.

Will have a go at it. Don’t expect super quick results but I feel positive that I should be able to navigate my way through it.

4 Likes

Hi, I also had a quick look. If you want to try a quick “hacky” proof-of-concept, there is a redirect mechanism in getrrdimage.cgi:
https://[ipfire]:444/cgi-bin/getrrdimage.cgi?origin=wiographs.cgi&graph=1&range=day
Should give you the graph of WIO host entry #1 - although getrrdimage doesn’t know anything about WIO.

Unfortunately WIOGraphs::wiographbox uses a fourth parameter ($hostname) which is not supported by Graphs::makegraphbox. A simple fix would be to load the hostname from config file in sub wiograph.

If you want to test this, modify wiographs.cgi:

  • Add require "${General::swroot}/graphs.pl";
  • Replace &WIOGraphs::wiographbox("wiographs.cgi","$hostid","day","$hostname"); with &Graphs::makegraphbox("wiographs.cgi","$hostid","day");
  • Remove the following line (print"<table...)

That should already be enough to remove the iframe and test the dark mode, but it will break the hostname header and trigger a HTTP 302 redirect.

2 Likes

Hi @cfusco, it’s a good question. I can at least open a bug report but to be honest, I will not fix this reported issue for this simple reason:

I’ll never put / write code in the project that will lower the security of IPFire or said differently, increase the available attack surface that already exist when using a web browser to display sensible information like our network / security settings.

I mean, one could write a browser extension that will read all the displayed text or form fields values and send them back to home (malicious actor).

Back to the main topic, I see no reason to fix my code to allow it to go across security boundaries in place in all browsers just to change the color of the RRD image. It would be a pure non-sense to me.

I hope to not have offended anyone by having such strict point of view. :bowing_man:

3 Likes

Hi @bonnietwin,

Thanks a lot for your inputs, they are always helpful!

As the original owner for that addon has stepped back from it after many, many years supporting it, I have picked that package up to maintain it.

Sorry about that, thanks a lot for maintaining it now! :+1:

I would have to figure out what iframes are and what alternative should be used.

Yes there is one and @luani already mentioned it. For what it worth I was going to say almost the same thing as @luani already did that in this case using an iframe was not necessary and simply redo what has been done for all other RRD generated images would be enough and then compatible with my work on the dark mode patch without any risky changes to be made from my side.

To warrant changes like that that my view would be that the dark mode would need to be submitted and accepted as a formal addon in IPFire rather than a third party addon that is not reviewed within the IPFire environment but requires IPFire to be changed to fit in with it.

Indeed, you’re absolutely right and it’s still on my todolist / roadmap. I’m just lacking time to do that yet but I’d definitely never ask for changes to be made on server side just for my own project! simply never. It would be a non-sense. I’ve just observed the difference between WIO RRD images and all others included by default and so replied to @tphz about why my patch didn’t worked on these graphs / RRD images.

As said to @cfusco, It’s kind of bug / issue where I’ll simply reply by “WON’T FIX” if it was in a ticketing system :sweat_smile: but of course with all the necessary explanations about why I won’t fix it.

Basically, the way I’m see things is the following: It’s at me to adapt to the IPFire web interface / structure and not the contrary. If I’m seeing something that not behave correctly I’ll simply create a bug report and discuss with the devs prior doing any changes in my own project.

I hope to have ensured you again that I have no will to put the IPFire project with my code. :bowing_man:

2 Likes

Hi @tphz,

Thanks a lot for these links! They were nice to read but they also rose some questions from my side.

  1. Did you observed a performance reduction when my patch is enabled on your side?
  2. Does the patch also works under Edge? (The chromium based version, not the older one)

I’ll try if I can improve all the defined CSS selectors in the patch and monitor the performances with the embedded GPU / usage monitoring tool in Chromium and see if some of the selectors (if not all) can be improved.

Aside of this and based on your given links, the current Javascript code is just used to do the following:

  1. Inject the theme selector icon: Required as this selector is not part of the original IPFire code
  2. Detect the current page: Required as some CSS code / style can’t be applied globally (on all pages)
  3. Inject required CSS code / style globally and conditionally

So the code does not try modify existing DOM elements. What may however impact the performances a little bit is the transition CSS effect that is applied over elements that has their colors changed by the injected CSS code.

I did that to make the changes less “brutal” / more “smooth” for the eyes but if all of you are considering that is useless and should be removed or made configurable, I can do it.

Just let me know :wink:

Hi @ummeegge,

Thanks a lot for your feedback! I’m glad it works correctly on your side now :+1: