From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: Test of cleanup branch
Date: Wed, 07 Aug 2024 15:50:46 +0100
Message-ID: <F662C347-8A02-4AE0-96E0-595F65AEABE0@ipfire.org>
In-Reply-To: <cc70b68c-d3ac-438d-ac1b-03cdefe290cf@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3921548830575393026=="
List-Id: <development.lists.ipfire.org>

--===============3921548830575393026==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello Adolf,

I just pushed a large number of changes and hopefully I have addressed everyt=
hing.

Please let me know :)

-Michael

> On 7 Aug 2024, at 13:58, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>=20
> Hi Michael,
>=20
> On 07/08/2024 14:19, Michael Tremer wrote:
>> Hello,
>> Thanks for looking at this again=E2=80=A6
> No problem. This is something that I definitely can help with.
>>> On 7 Aug 2024, at 11:56, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>>>=20
>>> Hi Michael,
>>>=20
>>> Yes, some of the problems have been fixed.
>>>=20
>>> On 06/08/2024 17:52, Michael Tremer wrote:
>>>> Hello,
>>>> I just pushed a bunch of changes that should hopefully resolve a few of =
the problems.
>>>> The only one that I can see remaining is that all sorts of form elements=
 (dropdowns, input boxes, etc.) are now 100% in width. They fill the entire h=
orizontal space. This is something I like. However, without the scaffolding a=
round being consistent, this creates a lot of problems. Sometimes we have a t=
able that is properly sized, sometimes we have I don=E2=80=99t even know what=
. I believe cleaning that up will be weeks worth of work because it is so fid=
dly. So, maybe I will revert that change and come up with a different solutio=
n for OpenVPN.
>>> For the DNS Server page you have the protocol entry box which is now the =
full width of the page but only has three letter acronyms to be entered. I do=
n't have a big problem with that but I suspect that there might be some pushb=
ack from some forum members.
>> I think I will revert that one change so that we don=E2=80=99t have 1000 n=
ew things to deal with...
> Sounds a good idea.
>>>> Please give the recent changes a test and let me know how it works.
>>> I made sure that I did a browser cache clear before reviewing the changes.
>> I bumped the version of the CSS sheet so that the browser thinks it is a n=
ew file:
>>   https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dcommitdiff;h=3D86ca826ff1=
33eb00b94ae47ac4ab27ac69f037c2
> I ran a CU186 version on my vm and then changed to the latest CU188 version=
 without doing the browser cache clear and it looks like it did previously wi=
th the cache clear so the new css version looks to have worked.
>>> The core programs section of the Services page now has colour on it for t=
he status of running or stopped.
>> Very good. It also has a very different layout. Do you think we need to la=
bel the third column in any way to make it clear that it is memory usage?
> I am not sure. It seems very obvious to me but not sure about all users.
>=20
> Only thing I thought of was to have the first column have the service names=
 centred. It is similar to the other two columns then and also places the nam=
es nearer to the status and memory entries, so is easier to see which one lin=
es up with which.
>>> The colours are not there for the status of any of the addons.
>> That I didn=E2=80=99t touch and clearly break. I did not have any adding i=
nstalled on the VM I set up for this. My bad.
>>> The Processes and Processes memory graphs that are normally on that page =
are now completely missing.
>> I removed those. I don=E2=80=99t think that there is any value in them. Th=
ey are incomplete, the graphs themselves miss units and legends and labelling=
. They either need major work or need to go. Let me know how you feel about i=
t.
> As you removed them deliberately then that is fine. From my point of view, =
I have always wondered exactly how I should interpret what they were showing =
to me. I don't think I ever really took much notice of them, so fine that the=
y are gone.
>>> Screenshot attached.
>>>=20
>>> The colour for the status is not showing for the IPSec and OpenVPN client=
 connections.
>> On the index page?
> On the index page it is fine. It is on the actual OpenVPN and IPSec pages. =
Attached is the OpenVPN screenshot for example.
>>> Also if you go to the Firewall Rules creation page the box to select Acce=
pt, Drop or Reject has no colour.
>> Meh=E2=80=A6 Thank you for finding all these things. I didn=E2=80=99t even=
 occur to me to look at that.
> That is what I am here for. Another colour thing for the top level Firewall=
 Rules page. Each rule has a line for the actual rule info and a second line =
for the remark. If the rule does not have a remark then there is no second li=
ne. The alternating grey shading is being done on each line rather than on th=
e rule and remark line combined so it ends up looking a bit weird.
> Also the colour coding for the drop, reject, accept that used to be at the =
start of each rule line is now missing. See attached screenshot.
>>> In the Services screenshot you can see that the Intrusion Prevention is s=
hown as Stopped but on the IPS page it is showing as running but with no colo=
ur. See second screenshot, if you look closely you can see that it shows RUNN=
ING for the status. I checked on the command line and Suricata is running so =
it is the Services page that has somehow got confused.
>> I forgot to cherry-pick the commit that uses the new widget for the IPS.
>> I fixed that now and will push shortly.
> I will test it when it arrives here.
>=20
> I also noticed that for the Samba addon, the top section for the status of =
the three daemons does not have any colour coding as it used to have. See att=
ached screenshot.
>=20
> Regards,
> Adolf.
>> -Michael
>>>=20
>>> Regards,
>>>=20
>>> Adolf.
>>>> -Michael
>>>>> On 3 Aug 2024, at 11:48, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>>>>>=20
>>>>> Hi Michael,
>>>>>=20
>>>>> On 02/08/2024 11:12, Michael Tremer wrote:
>>>>>> Hello Adolf,
>>>>>> Thank you for looking at this in depth :)
>>>>>>> On 1 Aug 2024, at 16:01, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>>>>>>>=20
>>>>>>> Hi Michael,
>>>>>>>=20
>>>>>>> Additional note written after all of the feedback below the dashed li=
ne was written. I did a shutdown of the vm with CU188 to check something in t=
he CU186 vm then went back to the CU188 version and things had changed. Most =
of the longer boxes are no longer longer and don't overlap other elements. Th=
e missing colours are back for IPSec, OpenVPN etc.
>>>>>> That is good news!
>>>>>=20
>>>>> In some later emails, I indicated that I found that the browser cache w=
as the reason that the colours came back. When I cleared my browser cache I w=
ent back to no colours and longer overlapping entry boxes.
>>>>>>> The graph period selection previously was in a horizontal line under =
the graph. Now it was still under the graph but it is now a vertical selectio=
n choice which can't have been intended as it takes far too much space. Howev=
er the key thing is that rebooting seems to be making things change, which sh=
ouldn't be happening. I have started to take screenshots of things to have so=
me evidence.
>>>>>> I think I have an explanation for this, and no, you are not going craz=
y. Since I have changed the CSS, it might be that the web UI is now rendering=
 some different HTML, but your browser has the old CSS cached and so those th=
ings don=E2=80=99t fit together any more.
>>>>>> This is a good example: https://git.ipfire.org/?p=3Dipfire-2.x.git;a=
=3Dcommitdiff;h=3Dfbfde0088c58c506cab80d23fc240e3cab863302
>>>>>> If you don=E2=80=99t have the changes from the CSS file, then you simp=
ly won=E2=80=99t see any colours here.
>>>>>=20
>>>>> Yes, I found that the browser cache was affecting things.
>>>>> However, when I clear the cache the result I get is the loss of the col=
ours in the services table page as an example or the overlapping of entry box=
es in the dhcp page as an example. I can get this effect consistently with do=
ing a fresh install and a browser cache clear. Have tested this out three tim=
es and always the same result. Longer boxes overlapping other elements and lo=
ss of status colours. See attached dhcp and services screenshots.
>>>>>> Some of the changes I implemented a user feedback - for example that t=
he headlines are hard to read on some devices that have not the best font ren=
dering. Other things are probably more of my own things. The graphs used to h=
ave a grey line around them. And then we had a box with another grey line aro=
und them. And then there was the big white box. It kind of reminded me of som=
e mirror cabinet.
>>>>>> Sometimes we have multiple levels of headlines that all say the same: =
Memory information -> Memory Graph -> Memory Usage per Day.
>>>>>>> Attached is a screenshot showing the graph period selectors which are=
 now vertically positioned at the bottom of the graphs.
>>>>>> So it is correct that the outer box is gone. The graph only has a head=
line when it needs one.
>>>>>> The Hour/Day/Week/=E2=80=A6 controls should however remain the same.
>>>>> That is definitely different. They are now boxes containing the words a=
t the bottom of the graph rather than just the words at the top of the graph =
and when first viewed all are grey (graph-example1) then become red when sele=
cted (graph-example2).
>>>>>>> Also attached is a screenshot of the pakfire page that is still missi=
ng the pak_ver numbers.
>>>>>> This is probably a bug from my changes in the tooling.
>>>>> I have seen the patch. Looks like that will fix it. Will test it to con=
firm, either with build and install or using nightly build once done.
>>>>>=20
>>>>>>> I don't know what is going on here.
>>>>>> Me pushing a lot of changes at a fast pace :)
>>>>> :-)
>>>>>>> ----------------------------------------------
>>>>>>>=20
>>>>>>> Okay, here is the feedback from reviewing a vm that actually included=
 the cleanup branch changes.
>>>>>>>=20
>>>>>>> The home and ssh items look better with the change. They now use alte=
rnating shades of grey to highlight each line.
>>>>>> This is now applied automatically to every table. Before, we used to h=
ave a lot of Perl magic to make that happen.
>>>>> That looks good.
>>>>>>> All graphs have new selection buttons for Hour, Day, Weekly etc that =
show in red when selected. This looks better. The default is the Day selectio=
n but when you start with a graph it does not indicate that it is using the D=
ay option, ie all buttons are showing grey.
>>>>>> Okay, this would be something that needs fixing then.
>>>>>>> Net-Traffic and ExtraHD have the same tables but instead of being cen=
tred on the page they are now on the Left Hand Side of the WUI page.
>>>>>>>=20
>>>>>>> The Zone Config page looks much busier now. Previously the dropdown b=
ox for native or vlan selection was next to the vlan id box and the space was=
 good enough for that. Now they are placed one above the other, so wider, whi=
ch is not needed but they now fill the whole space of the section before the =
next nic interface starts. I think the previous version looked clearer.
>>>>>>>=20
>>>>>>> The IPS page and the Services page no longer show the red or green ba=
ckground for running or stopped. You still see the words but they are now in =
white on a light grey background or white on a slightly darker grey.
>>>>>>>=20
>>>>>>> On the Services page the first graph which should be titled Processes=
 is labelled 100%.
>>>>>>>=20
>>>>>>> On the System page the graphs have no name at all. On the vm I used t=
hey should have been labelled CPU and Load Avg.
>>>>>>>=20
>>>>>>> On the Pakfire page the list of available or installed addons do not =
show the pak_ver number so that you get alsa-1.2.10- instead of alsa-1.2.10-20
>>>>>>>=20
>>>>>>> The IPSec and OpenVPN pages no longer show the green, red or blue col=
ours for the connection status.
>>>>>>>=20
>>>>>>> On the Firewall Rules page the table has no colours for Accept, Rejec=
t or Drop for any of the rules. The alternate grey shading has got mixed up a=
s it is considering that if there is a remark line for a firewall rule that i=
s a separate line and the grey shading has to be changed. Definitely not righ=
t. Some of the Destinations have not had the right colour code applied for th=
e zone colour - left grey. The boxes giving the policy applied for each firew=
all section are not colour coded for Allow or Blocked.
>>>>>>>=20
>>>>>>> On the actual firewall creation page, there are longer boxes that ove=
rlap labels or other elements. There is no colour in the Drop, Reject, Accept=
 selection box just one shade of grey.
>>>>>>>=20
>>>>>>> The vulnerabilities page has the left hand section now in black backg=
round with white lettering. Different but probably okay.
>>>>>> Okay, this is all very useful. I will install a machine and go through=
 all of this and see what I can fix.
>>>>>>> The following menu items showed longer entry boxes that overlap label=
s or other elements
>>>>>>> DHCP
>>>>>>> DNS Forward
>>>>>>> Static Routes
>>>>>>> Wake on LAN
>>>>>>> Time Server
>>>>>>> Log Settings
>>>>>>> Proxy Logs
>>>>>>>=20
>>>>>>>=20
>>>>>>> The following menu items showed longer entry boxes that did not overl=
ap other elements but you end up with a much longer box length than is needed=
 for a day number for example
>>>>>>> Firewall Groups
>>>>>>> Firewall Options
>>>>>>> Blue Access
>>>>>>> Log Summary
>>>>>>> Dynamic DNS
>>>>>>> Proxy reports
>>>>>>> Firewall logs
>>>>>>> Firewall graphs - IP, Port & Country
>>>>>>> IPS Logs
>>>>>>> IP Address blocklist logs
>>>>>>> OpenVPN RW logs
>>>>>>> URL Filter Logs
>>>>>>> System Logs
>>>>>>> Captive portal
>>>>>>> Connection Scheduler
>>>>>>> Assign MAC Address
>>>>>>>=20
>>>>>>>=20
>>>>>>> The following menu items showed no change/issues except maybe for the=
 graph period selection buttons .
>>>>>>> Backup
>>>>>>> Shutdown
>>>>>>> Credits
>>>>>>> Mailservice
>>>>>>> Memory
>>>>>>> Media
>>>>>>> Network External, Internal and Other
>>>>>>> OpenVPN RW and N2N Statistics
>>>>>>> WIO
>>>>>>> Hardware graphs
>>>>>>> Connections
>>>>>>> Mdstat
>>>>>>> DNS
>>>>>>> Web Proxy
>>>>>>> URL Filter
>>>>>>> Update Accelerator
>>>>>>> Edit Hosts
>>>>>>> QOS
>>>>>>> IP Address Blocklist
>>>>>>> Location Block
>>>>>>> IPTables
>>>>>>>=20
>>>>>>> I am really sorry for giving false hope that the cleanup branch had g=
one really well.
>>>>>> Well, it happens :) That is why next is called next.
>>>>> Yes but me providing the original first review feedback on a vm system =
that did not have the cleanup changes actually in it, really shouldn't have h=
appened.
>>>>>=20
>>>>> Regards,
>>>>> Adolf.
>>>>>> -Michael
>>>>>>>=20
>>>>>>> Regards,
>>>>>>>=20
>>>>>>> Adolf.
>>>>>>>=20
>>>>>>> On 01/08/2024 12:50, Adolf Belka wrote:
>>>>>>>> Hi Michael,
>>>>>>>>=20
>>>>>>>> On 25/07/2024 12:28, Adolf Belka wrote:
>>>>>>>>> Hi Michael,
>>>>>>>>>=20
>>>>>>>>> On 25/07/2024 10:44, Michael Tremer wrote:
>>>>>>>>>> Hello Adolf,
>>>>>>>>>>=20
>>>>>>>>>> Thank you for getting back on this so quickly.
>>>>>>>>>>=20
>>>>>>>> Maybe I got back too quickly!!
>>>>>>>>=20
>>>>>>>> I have just done a build with next for a bugfix patch and installed =
it into my vm and the pages look quite different with some things missing, li=
ke the green and red colours for if services are running or not and some grap=
h titles are incorrect.
>>>>>>>>=20
>>>>>>>> I took the iso from the latest directory in the nightly build but it=
 looks like it was still linked to the previous version when I downloaded it =
as the b2 sum is for build 6460dbbf from the day before.
>>>>>>>>=20
>>>>>>>> Downloading the iso today from the latest directory gives me the bui=
ld ed2c97b7.
>>>>>>>>=20
>>>>>>>> I am going to install that version now but I suspect it will show th=
e same as I found with my build.
>>>>>>>>=20
>>>>>>>> So it looks like my original review was based on the version before =
the cleanup branch changes were included.
>>>>>>>>=20
>>>>>>>> I will come back with new feedback of what I find from the latest br=
anch that I have now downloaded.
>>>>>>>>=20
>>>>>>>> Sorry. In future maybe I should wait till the following day, or down=
load from the build named directory instead of the directory named latest.
>>>>>>>>=20
>>>>>>>> Regards,
>>>>>>>>=20
>>>>>>>> Adolf
>>>>>>>>=20
>>>>>>>>>> I suppose this also means that the ISO in the next branch boots ju=
st fine, too?
>>>>>>>>>=20
>>>>>>>>> Yes, that is correct. I used the iso from the nightly next latest d=
irectory. No problems with the install at all.
>>>>>>>>> This tested out raid and all 4 interfaces.
>>>>>>>>>=20
>>>>>>>>> Regards,
>>>>>>>>> Adolf.
>>>>>>>>>=20
>>>>>>>>>>=20
>>>>>>>>>> Best,
>>>>>>>>>> -Michael
>>>>>>>>>>=20
>>>>>>>>>>> On 25 Jul 2024, at 09:07, Adolf Belka <adolf.belka(a)ipfire.org> =
wrote:
>>>>>>>>>>>=20
>>>>>>>>>>> Hi Michael,
>>>>>>>>>>>=20
>>>>>>>>>>> I installed CU188 from the nightly onto a vm system and looked th=
ough all the WUI pages and everything looked fine.
>>>>>>>>>>>=20
>>>>>>>>>>> I then restored a CU187 backup from my testing and then checked t=
he OpenVPN RW and N2N. Both worked fine with no problems. Also the logging fo=
r that all worked with no problems.
>>>>>>>>>>>=20
>>>>>>>>>>> Also checked out the IPS, IP Blocklists, Firewall groups and rule=
s, DNS ...
>>>>>>>>>>>=20
>>>>>>>>>>> Everything I have looked at shows no impact from the cleanup bran=
ch changes. It looks good to me.
>>>>>>>>>>>=20
>>>>>>>>>>> Of course good for others to also evaluate.
>>>>>>>>>>>=20
>>>>>>>>>>> Regards,
>>>>>>>>>>>=20
>>>>>>>>>>> Adolf.
>>>>>>>>>>>=20
>>>>>>> <Screenshot_2024-08-01_16-45-00.png><Screenshot_2024-08-01_16-47-11.p=
ng>
>>>>> <dhcp.png><services.png><graph-example1.png><graph-example2.png>
>>> <Services_page_2024-08-07_12-44-08.png><IPS_page_2024-08-07_12-49-33.png>
> <OpenVPN_page_2024-08-07_14-39-58.png><Firewall_rules_2024-08-07_14-51-48.p=
ng><Samba_page_2024-08-07_14-40-58.png>


--===============3921548830575393026==--