diff mbox series

[1/4] zoneconf.cgi: Clean up HTML output

Message ID 20201117062904.1547-1-hofmann@leo-andres.de
State Accepted
Commit 96d0c76154fe1b52796517afa7afe23f1d888638
Headers
Series [1/4] zoneconf.cgi: Clean up HTML output |

Commit Message

Leo-Andres Hofmann Nov. 17, 2020, 6:29 a.m. UTC 
  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(-)

Comments

Michael Tremer Nov. 17, 2020, 10:54 a.m. UTC | #1 
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
>
Peter Müller Dec. 1, 2020, 10:25 p.m. UTC | #2 
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
diff mbox series

Patch

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 = "";