This adds missing brackets, cleans up the indentation and removes unnecessary CSS.
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- html/cgi-bin/zoneconf.cgi | 46 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index d99a3e611..067410582 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -38,10 +38,6 @@ my $css = <<END height: 4em; }
- tr.thin { - height: 3em; - } - td.narrow { width: 11em; } @@ -85,10 +81,6 @@ my $css = <<END border-left-style: none; }
- td.disabled { - background-color: #cccccc; - } - td.textcenter { text-align: center; } @@ -312,8 +304,8 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { print <<END <form method='post' enctype='multipart/form-data'> <table> - <tr> - <td class="h narrow topleft" /td> + <tr> + <td class="h narrow topleft"></td> END ;
@@ -332,7 +324,7 @@ foreach (@zones) { my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));
if ($red_restricted) { - print "<td class='h textcenter $_'>$uc ($red_type)</td>"; + print "\t\t<td class='h textcenter $_'>$uc ($red_type)</td>\n";
next; # We're done here } @@ -350,7 +342,7 @@ foreach (@zones) { }
print <<END - <td class='h textcenter $_'>$uc</br> + <td class='h textcenter $_'>$uc<br> <select name="MODE $uc"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option> @@ -361,7 +353,7 @@ END ; }
-print "</tr>"; +print "\t</tr>\n";
my $slightlygrey = "";
@@ -370,7 +362,8 @@ foreach (@nics) { my $nic = $_->[1]; my $wlan = $_->[2];
- print "<tr><td class='h narrow textcenter'>$nic<br>$mac</td>"; + print "\t<tr>\n"; + print "\t\t<td class='h narrow textcenter'>$nic<br>$mac</td>\n";
# Iterate through all zones and check if the current NIC is assigned to it foreach (@zones) { @@ -393,7 +386,12 @@ foreach (@nics) { $checked = "checked"; }
- print "<td class='textcenter $slightlygrey'><input type='radio' id='PPPACCESS $mac' name='PPPACCESS' value='$mac' $checked></td>"; + print <<END + <td class="textcenter $slightlygrey"> + <input type="radio" id="PPPACCESS $mac" name="PPPACCESS" value="$mac" $checked> + </td> +END +; next; # We're done here } } @@ -432,19 +430,19 @@ foreach (@nics) { my $vlan_disabled = ($wlan) ? "disabled" : "";
print <<END - <td class="textcenter $slightlygrey"> - <select name="ACCESS $uc $mac" onchange="document.getElementById('TAG $uc $mac').disabled = (this.value === 'VLAN' ? false : true)"> - <option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option> - <option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option> - <option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option> - </select> - <input type="number" class="vlanid" id="TAG $uc $mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled> - </td> + <td class="textcenter $slightlygrey"> + <select name="ACCESS $uc $mac" onchange="document.getElementById('TAG $uc $mac').disabled = (this.value === 'VLAN' ? false : true)"> + <option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option> + <option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option> + <option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option> + </select> + <input type="number" class="vlanid" id="TAG $uc $mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled> + </td> END ; }
- print "</tr>"; + print "\t</tr>\n";
if ($slightlygrey) { $slightlygrey = "";
This fixes two minor violations of the HTML standard: - <a> elements may not contain nested <button> elements: Replace the button with a simple hyperlink, because it was only used as a link anyway.
- "id" attributes may not contain whitespace: Remove unneeded attribute, use hyphens instead of spaces.
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- html/cgi-bin/zoneconf.cgi | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 067410582..2346aa829 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -99,10 +99,6 @@ my $css = <<END #submit-container.input { margin-left: auto; } - - button { - margin-top: 1em; - } </style> END ; @@ -282,7 +278,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { if ($VALIDATE_error) { &Header::openbox('100%', 'left', $Lang::tr{"error"});
- print "$VALIDATE_error<br><a href='/cgi-bin/zoneconf.cgi'><button>$Lang::tr{'ok'}</button></a>"; + print "$VALIDATE_error<br><br><a href='$ENV{'SCRIPT_NAME'}'>$Lang::tr{'back'}</a>\n";
&Header::closebox(); &Header::closebigbox(); @@ -388,7 +384,7 @@ foreach (@nics) {
print <<END <td class="textcenter $slightlygrey"> - <input type="radio" id="PPPACCESS $mac" name="PPPACCESS" value="$mac" $checked> + <input type="radio" name="PPPACCESS" value="$mac" $checked> </td> END ; @@ -431,12 +427,12 @@ END
print <<END <td class="textcenter $slightlygrey"> - <select name="ACCESS $uc $mac" onchange="document.getElementById('TAG $uc $mac').disabled = (this.value === 'VLAN' ? false : true)"> + <select name="ACCESS $uc $mac" onchange="document.getElementById('TAG-$uc-$mac').disabled = (this.value === 'VLAN' ? false : true)"> <option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option> <option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option> <option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option> </select> - <input type="number" class="vlanid" id="TAG $uc $mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled> + <input type="number" class="vlanid" id="TAG-$uc-$mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled> </td> END ;
- Add an element id so that the styling only affects the zone table - Alternating row colors are now generated by CSS, remove unneeded Perl code
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- html/cgi-bin/zoneconf.cgi | 73 ++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 40 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 2346aa829..ef361af95 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -28,63 +28,64 @@ require "${General::swroot}/header.pl";
my $css = <<END <style> - table { + table#zoneconf { width: 100%; border-collapse: collapse; table-layout: fixed; }
- tr { + #zoneconf tr { height: 4em; }
- td.narrow { - width: 11em; + #zoneconf td { + padding: 5px 10px; + border: 0.5px solid black; + text-align: center; }
- td { - padding: 5px; - padding-left: 10px; - padding-right: 10px; - border: 0.5px solid black; + /* dark grey header cells */ + #zoneconf td.heading { + background-color: grey; + color: white; + } + #zoneconf td.heading::first-line { + font-weight: bold; + line-height: 1.6; }
- td.slightlygrey { - background-color: #F0F0F0; + /* narrow left column */ + #zoneconf tr > td:first-child { + width: 11em; }
- td.h { - background-color: grey; - color: white; - font-weight: 800; + /* alternating row background color */ + #zoneconf tr:nth-child(2n+3) { + background-color: #F0F0F0; }
- td.green { + #zoneconf td.green { background-color: $Header::colourgreen; }
- td.red { + #zoneconf td.red { background-color: $Header::colourred; }
- td.blue { + #zoneconf td.blue { background-color: $Header::colourblue; }
- td.orange { + #zoneconf td.orange { background-color: $Header::colourorange; }
- td.topleft { - background-color: white; + #zoneconf td.topleft { + background-color: $Header::pagecolour; border-top-style: none; border-left-style: none; }
- td.textcenter { - text-align: center; - } - input.vlanid { width: 4em; } @@ -299,9 +300,9 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
print <<END <form method='post' enctype='multipart/form-data'> - <table> + <table id="zoneconf"> <tr> - <td class="h narrow topleft"></td> + <td class="topleft"></td> END ;
@@ -320,7 +321,7 @@ foreach (@zones) { my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));
if ($red_restricted) { - print "\t\t<td class='h textcenter $_'>$uc ($red_type)</td>\n"; + print "\t\t<td class='heading $_'>$uc ($red_type)</td>\n";
next; # We're done here } @@ -338,7 +339,7 @@ foreach (@zones) { }
print <<END - <td class='h textcenter $_'>$uc<br> + <td class='heading $_'>$uc<br> <select name="MODE $uc"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option> @@ -351,15 +352,13 @@ END
print "\t</tr>\n";
-my $slightlygrey = ""; - foreach (@nics) { my $mac = $_->[0]; my $nic = $_->[1]; my $wlan = $_->[2];
print "\t<tr>\n"; - print "\t\t<td class='h narrow textcenter'>$nic<br>$mac</td>\n"; + print "\t\t<td class='heading'>$nic<br>$mac</td>\n";
# Iterate through all zones and check if the current NIC is assigned to it foreach (@zones) { @@ -383,7 +382,7 @@ foreach (@nics) { }
print <<END - <td class="textcenter $slightlygrey"> + <td class=""> <input type="radio" name="PPPACCESS" value="$mac" $checked> </td> END @@ -426,7 +425,7 @@ END my $vlan_disabled = ($wlan) ? "disabled" : "";
print <<END - <td class="textcenter $slightlygrey"> + <td class=""> <select name="ACCESS $uc $mac" onchange="document.getElementById('TAG-$uc-$mac').disabled = (this.value === 'VLAN' ? false : true)"> <option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option> <option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option> @@ -439,12 +438,6 @@ END }
print "\t</tr>\n"; - - if ($slightlygrey) { - $slightlygrey = ""; - } else { - $slightlygrey = "slightlygrey"; - } }
print <<END
This improves the usability of the zone configuration by marking assigned NICs in the zone color. The highlighting is initially applied to the static HTML output, and JavaScript is used to follow changes made by the user.
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- config/rootfiles/common/web-user-interface | 1 + html/cgi-bin/zoneconf.cgi | 21 +++++--- html/html/include/zoneconf.js | 56 ++++++++++++++++++++++ 3 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 html/html/include/zoneconf.js
diff --git a/config/rootfiles/common/web-user-interface b/config/rootfiles/common/web-user-interface index 44856fcc2..3eac4411a 100644 --- a/config/rootfiles/common/web-user-interface +++ b/config/rootfiles/common/web-user-interface @@ -308,6 +308,7 @@ srv/web/ipfire/html/images/wakeup.gif srv/web/ipfire/html/images/window-new.png srv/web/ipfire/html/include srv/web/ipfire/html/include/snortupdateutility.js +srv/web/ipfire/html/include/zoneconf.js srv/web/ipfire/html/index.cgi srv/web/ipfire/html/redirect-templates srv/web/ipfire/html/redirect-templates/legacy diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index ef361af95..0914ceb78 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -26,7 +26,7 @@ require '/var/ipfire/general-functions.pl'; require "${General::swroot}/lang.pl"; require "${General::swroot}/header.pl";
-my $css = <<END +my $extraHead = <<END <style> table#zoneconf { width: 100%; @@ -101,6 +101,8 @@ my $css = <<END margin-left: auto; } </style> + +<script src="/include/zoneconf.js"></script> END ;
@@ -151,7 +153,7 @@ foreach (@nics) { } }
-&Header::openpage($Lang::tr{"zoneconf title"}, 1, $css); +&Header::openpage($Lang::tr{"zoneconf title"}, 1, $extraHead); &Header::openbigbox('100%', 'center');
### Evaluate POST parameters ### @@ -364,6 +366,7 @@ foreach (@nics) { foreach (@zones) { my $uc = uc $_; my $dev_name = $ethsettings{"${uc}_DEV"}; + my $highlight = "";
if ($dev_name eq "") { # Again, skip the zone if it is not activated next; @@ -379,11 +382,12 @@ foreach (@nics) {
if ($mac eq $ethsettings{"${uc}_MACADDR"}) { $checked = "checked"; + $highlight = $_; }
print <<END - <td class=""> - <input type="radio" name="PPPACCESS" value="$mac" $checked> + <td class="$highlight"> + <input type="radio" name="PPPACCESS" value="$mac" data-zone="RED" data-mac="$mac" onchange="highlightAccess(this)" $checked> </td> END ; @@ -424,9 +428,14 @@ END $access_selected{"NONE"} = ($access_selected{"NATIVE"} eq "") && ($access_selected{"VLAN"} eq "") ? "selected" : ""; my $vlan_disabled = ($wlan) ? "disabled" : "";
+ # If the interface is assigned, hightlight table cell + if ($access_selected{"NONE"} eq "") { + $highlight = $_; + } + print <<END - <td class=""> - <select name="ACCESS $uc $mac" onchange="document.getElementById('TAG-$uc-$mac').disabled = (this.value === 'VLAN' ? false : true)"> + <td class="$highlight"> + <select name="ACCESS $uc $mac" data-zone="$uc" data-mac="$mac" onchange="highlightAccess(this)"> <option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option> <option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option> <option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option> diff --git a/html/html/include/zoneconf.js b/html/html/include/zoneconf.js new file mode 100644 index 000000000..d76f0ab68 --- /dev/null +++ b/html/html/include/zoneconf.js @@ -0,0 +1,56 @@ +/*############################################################################# +# # +# IPFire.org - A linux based firewall # +# Copyright (C) 2007-2020 IPFire Team info@ipfire.org # +# # +# This program is free software: you can redistribute it and/or modify # +# it under the terms of the GNU General Public License as published by # +# the Free Software Foundation, either version 3 of the License, or # +# (at your option) any later version. # +# # +# This program is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty of # +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # +# GNU General Public License for more details. # +# # +# You should have received a copy of the GNU General Public License # +# along with this program. If not, see http://www.gnu.org/licenses/. # +# # +#############################################################################*/ + +//zoneconf.cgi dynamic highlighting of interface access selection +//(call this from "onchange" event of selection elements) +function highlightAccess(selectObj) { + if(!(selectObj && ('zone' in selectObj.dataset) && ('mac' in selectObj.dataset))) { + return; //required parameters are missing + } + + var zoneColor = selectObj.dataset.zone.trim().toLowerCase(); //zone color (red/green/blue/orange) CSS class + + function colorParentCell(obj, color, enabled = true) { //find nearest parent table cell of "obj" and set its CSS color class + do { + obj = obj.parentElement; + } while(obj && (obj.nodeName.toUpperCase() !== 'TD')); + if(obj && (['green', 'red', 'orange', 'blue'].includes(color))) { + obj.classList.toggle(color, enabled); + } + } + + //handle other associated input fields + if(selectObj.type.toUpperCase() === 'RADIO') { //this is a radio button group: clear all highlights + let radios = document.getElementsByName(selectObj.name); + radios.forEach(function(button) { + if(button.nodeName.toUpperCase() === 'INPUT') { //make sure the found element is a button + colorParentCell(button, zoneColor, false); //remove css + } + }); + } else { //this is a dropdown menu: enable/disable additional VLAN tag input box + let tagInput = document.getElementById('TAG-' + selectObj.dataset.zone + '-' + selectObj.dataset.mac); //tag input element selector + if(tagInput) { + tagInput.disabled = (selectObj.value !== 'VLAN'); //enable tag input if VLAN mode selected + } + } + + //if interface is assigned, highlight table cell in zone color + colorParentCell(selectObj, zoneColor, (selectObj.value !== 'NONE')); +}
Hello Leo,
This looks like a very clean set of patches. Well done.
I haven’t tested it, but I have seen the previous version in action.
It really looks good.
Best, -Michael
Reviewed-by: Michael Tremer michael.tremer@ipfire.org
On 17 Nov 2020, at 06:29, Leo-Andres Hofmann hofmann@leo-andres.de wrote:
This adds missing brackets, cleans up the indentation and removes unnecessary CSS.
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de
html/cgi-bin/zoneconf.cgi | 46 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index d99a3e611..067410582 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -38,10 +38,6 @@ my $css = <<END height: 4em; }
- tr.thin {
height: 3em;- }
- td.narrow { width: 11em; }
@@ -85,10 +81,6 @@ my $css = <<END border-left-style: none; }
- td.disabled {
background-color: #cccccc;- }
- td.textcenter { text-align: center; }
@@ -312,8 +304,8 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { print <<END
<form method='post' enctype='multipart/form-data'> <table> - <tr> - <td class="h narrow topleft" /td> + <tr> + <td class="h narrow topleft"></td> END ;
@@ -332,7 +324,7 @@ foreach (@zones) { my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));
if ($red_restricted) {
print "<td class='h textcenter $_'>$uc ($red_type)</td>";
print "\t\t<td class='h textcenter $_'>$uc ($red_type)</td>\n"; next; # We're done here}
@@ -350,7 +342,7 @@ foreach (@zones) { }
print <<END
<td class='h textcenter $_'>$uc</br>
<td class='h textcenter $_'>$uc<br> <select name="MODE $uc"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option>@@ -361,7 +353,7 @@ END ; }
-print "</tr>"; +print "\t</tr>\n";
my $slightlygrey = "";
@@ -370,7 +362,8 @@ foreach (@nics) { my $nic = $_->[1]; my $wlan = $_->[2];
- print "<tr><td class='h narrow textcenter'>$nic<br>$mac</td>";
print "\t<tr>\n";
print "\t\t<td class='h narrow textcenter'>$nic<br>$mac</td>\n";
# Iterate through all zones and check if the current NIC is assigned to it foreach (@zones) {
@@ -393,7 +386,12 @@ foreach (@nics) { $checked = "checked"; }
print "<td class='textcenter $slightlygrey'><input type='radio' id='PPPACCESS $mac' name='PPPACCESS' value='$mac' $checked></td>";
print <<END<td class="textcenter $slightlygrey"><input type="radio" id="PPPACCESS $mac" name="PPPACCESS" value="$mac" $checked></td>+END +; next; # We're done here } } @@ -432,19 +430,19 @@ foreach (@nics) { my $vlan_disabled = ($wlan) ? "disabled" : "";
print <<END
<td class="textcenter $slightlygrey"><select name="ACCESS $uc $mac" onchange="document.getElementById('TAG $uc $mac').disabled = (this.value === 'VLAN' ? false : true)"><option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option><option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option><option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option></select><input type="number" class="vlanid" id="TAG $uc $mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled></td>
<td class="textcenter $slightlygrey"><select name="ACCESS $uc $mac" onchange="document.getElementById('TAG $uc $mac').disabled = (this.value === 'VLAN' ? false : true)"><option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option><option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option><option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option></select><input type="number" class="vlanid" id="TAG $uc $mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled></td>END ; }
- print "</tr>";
print "\t</tr>\n";
if ($slightlygrey) { $slightlygrey = "";
-- 2.27.0.windows.1
Hello Leo-Andreas,
sorry for replying late on this; I just wanted to say Hi and thank you very much for your efforts on cleaning up this CGI - and perhaps others in the future. :-)
Unfortunately, some of those CGIs are not really in a good shape and do things in a - let's put it this way - non-optimal fashion. Within the past years, there was never enough time to tidy them up one by one.
In case I have understood your messages to that effect that you are willing to change this, I absolutely look forward to it and applaud this intent.
Please ignore the noise if this is a misunderstanding.
Thanks, and best regards, Peter Müller
-
Leo-Andres Hofmann -
Michael Tremer -
Peter Müller