fix: avoid mutating original server array and add null guard

Address code review findings:
- Sort a shallow copy instead of mutating the caller's array
- Add null guard on server.name to handle malformed entries
- Use original SPEEDTEST_SERVERS index for classic UI option values
This commit is contained in:
valer23 2026-04-17 10:28:38 +02:00 committed by sstidl
parent 4ec3a755fa
commit 42d700cf08
2 changed files with 24 additions and 20 deletions

View file

@ -231,18 +231,20 @@ function populateDropdown(servers) {
}
// Sort servers by country, then by city within the same country
servers.sort((a, b) => {
const commaA = a.name.lastIndexOf(",");
const commaB = b.name.lastIndexOf(",");
const countryA = commaA >= 0 ? a.name.substring(commaA + 1).trim() : a.name;
const countryB = commaB >= 0 ? b.name.substring(commaB + 1).trim() : b.name;
const cityA = commaA >= 0 ? a.name.substring(0, commaA).trim() : "";
const cityB = commaB >= 0 ? b.name.substring(0, commaB).trim() : "";
const sorted = [...servers].sort((a, b) => {
const nameA = a.name || "";
const nameB = b.name || "";
const commaA = nameA.lastIndexOf(",");
const commaB = nameB.lastIndexOf(",");
const countryA = commaA >= 0 ? nameA.substring(commaA + 1).trim() : nameA;
const countryB = commaB >= 0 ? nameB.substring(commaB + 1).trim() : nameB;
const cityA = commaA >= 0 ? nameA.substring(0, commaA).trim() : "";
const cityB = commaB >= 0 ? nameB.substring(0, commaB).trim() : "";
return countryA.localeCompare(countryB) || cityA.localeCompare(cityB);
});
// Populate the list to choose from
servers.forEach((server) => {
sorted.forEach((server) => {
const item = document.createElement("li");
const link = document.createElement("a");
link.href = "#";

View file

@ -51,22 +51,24 @@
if (server != null) { //at least 1 server is available
I("loading").className = "hidden"; //hide loading message
//sort servers by country, then by city
SPEEDTEST_SERVERS.sort(function (a, b) {
var commaA = a.name.lastIndexOf(",");
var commaB = b.name.lastIndexOf(",");
var countryA = commaA >= 0 ? a.name.substring(commaA + 1).trim() : a.name;
var countryB = commaB >= 0 ? b.name.substring(commaB + 1).trim() : b.name;
var cityA = commaA >= 0 ? a.name.substring(0, commaA).trim() : "";
var cityB = commaB >= 0 ? b.name.substring(0, commaB).trim() : "";
var sortedServers = SPEEDTEST_SERVERS.slice().sort(function (a, b) {
var nameA = a.name || "";
var nameB = b.name || "";
var commaA = nameA.lastIndexOf(",");
var commaB = nameB.lastIndexOf(",");
var countryA = commaA >= 0 ? nameA.substring(commaA + 1).trim() : nameA;
var countryB = commaB >= 0 ? nameB.substring(commaB + 1).trim() : nameB;
var cityA = commaA >= 0 ? nameA.substring(0, commaA).trim() : "";
var cityB = commaB >= 0 ? nameB.substring(0, commaB).trim() : "";
return countryA.localeCompare(countryB) || cityA.localeCompare(cityB);
});
//populate server list for manual selection
for (var i = 0; i < SPEEDTEST_SERVERS.length; i++) {
if (SPEEDTEST_SERVERS[i].pingT == -1) continue;
for (var i = 0; i < sortedServers.length; i++) {
if (sortedServers[i].pingT == -1) continue;
var option = document.createElement("option");
option.value = i;
option.textContent = SPEEDTEST_SERVERS[i].name;
if (SPEEDTEST_SERVERS[i] === server) option.selected = true;
option.value = SPEEDTEST_SERVERS.indexOf(sortedServers[i]);
option.textContent = sortedServers[i].name;
if (sortedServers[i] === server) option.selected = true;
I("server").appendChild(option);
}
//show test UI