Code Improvements

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

UPDATE:
In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

[TheS1R]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

The validation for the Set Directory for F/W Updates appears to work great! One small glitch (not necessarily needing to be changed) is that the same error dialogue is presented a second time after dismissing it for invalid path.

[ExtremeFiretop]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported.
Afterwards the F12 menu started flooding with these:

I did run MerlinAU before testing to load the new /CheckHelper.js

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.
UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

The validation for the Set Directory for F/W Updates appears to work great! One small glitch (not necessarily needing to be changed) is that the same error dialogue is presented a second time after dismissing it for invalid path.

I see you removed the comment about being unable to have a "mount point" path in the "F/W Update" directory path text field so I assume you're all good, and it was just due to a user error, correct? Or, was it something else?

I didn't get (and still don't get) a duplicate error dialog box during my testing & validation runs, but I suspect the issue might be due to some timing between different browser clients and the HTTP web server module on your specific ASUS router. Also, WebGUI function calls run asynchronously, especially relative to the shell script so that also may be a factor.

Let me try something, while I wait for dinner...

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported. Afterwards the F12 menu started flooding with these:

I did run MerlinAU before testing to load the new /CheckHelper.js

Interesting. My runs are very clean - no errors at all.

[TheS1R]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.
UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

The validation for the Set Directory for F/W Updates appears to work great! One small glitch (not necessarily needing to be changed) is that the same error dialogue is presented a second time after dismissing it for invalid path.

I see you removed the comment about being unable to have a "mount point" path in the "F/W Update" directory path text field so I assume you're all good, and it was just due to a user error, correct? Or, was it something else?

II's good as it is. /mnt is valid as well /tmp/mnt, but it's not worth the code to allow it.

I didn't get (and still don't get) a duplicate error dialog box during my testing & validation runs, but I suspect the issue might be due to some timing between different browser clients and the HTTP web server module on your specific ASUS router. Also, WebGUI function calls run asynchronously, especially relative to the shell script so that also may be a factor.

Timing issue certainly possible. @ExtremeFiretop and I have same GT-BE98 Pro.

Let me try something, while I wait for dinner...

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported.

I just made a simple change in the shell script (just to test a suspicion at this point). Load this updated version and try the WebGUI page again to see if you get duplicate error dialog boxes.

[TheS1R]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported.

I just made a simple change in the shell script (just to test a suspicion at this point). Load this updated version and try the WebGUI page again to see if you get duplicate error dialog boxes.

No difference — I still get duplication of the error dialog.

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.
UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

The validation for the Set Directory for F/W Updates appears to work great! One small glitch (not necessarily needing to be changed) is that the same error dialogue is presented a second time after dismissing it for invalid path.

I see you removed the comment about being unable to have a "mount point" path in the "F/W Update" directory path text field so I assume you're all good, and it was just due to a user error, correct? Or, was it something else?

II's good as it is. /mnt is valid as well /tmp/mnt, but it's not worth the code to allow it.

I'm confused by your statement. If both are currently valid and allowed, what else is needed for the code to "allow it."??

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported.

I just made a simple change in the shell script (just to test a suspicion at this point). Load this updated version and try the WebGUI page again to see if you get duplicate error dialog boxes.

No difference — I still get duplication of the error dialog.

So it looks like the validation function is being triggered twice in a row in a very short time, but only on your specific router. I need to think about this a bit longer...

[TheS1R]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.
UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

The validation for the Set Directory for F/W Updates appears to work great! One small glitch (not necessarily needing to be changed) is that the same error dialogue is presented a second time after dismissing it for invalid path.

I see you removed the comment about being unable to have a "mount point" path in the "F/W Update" directory path text field so I assume you're all good, and it was just due to a user error, correct? Or, was it something else?

II's good as it is. /mnt is valid as well /tmp/mnt, but it's not worth the code to allow it.

I'm confused by your statement. If both are currently valid and allowed, what else is needed for the code to "allow it."??

You are correct. My mistake — it is good as is after retrying.

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.
UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

The validation for the Set Directory for F/W Updates appears to work great! One small glitch (not necessarily needing to be changed) is that the same error dialogue is presented a second time after dismissing it for invalid path.

I see you removed the comment about being unable to have a "mount point" path in the "F/W Update" directory path text field so I assume you're all good, and it was just due to a user error, correct? Or, was it something else?

II's good as it is. /mnt is valid as well /tmp/mnt, but it's not worth the code to allow it.

I'm confused by your statement. If both are currently valid and allowed, what else is needed for the code to "allow it."??

You are correct. My mistake — it is good as is after retrying.

OK, so we're all good on this particular syntax check.

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported.

I just made a simple change in the shell script (just to test a suspicion at this point). Load this updated version and try the WebGUI page again to see if you get duplicate error dialog boxes.

No difference — I still get duplication of the error dialog.

Just to be clear. What you observed is that the 2nd error dialog box is displayed immediately after clicking the OK button to dismiss the first one, correct?

If the above scenario is not the case, what exactly are you seeing?

What happens if you wait 4-5 seconds before dismissing the first error box?
Just trying to pinpoint if it's indeed a timing issue.

[ExtremeFiretop]

Sorry guys, the girlfriend is calling for dinner.
Will be back shortly.

To answer your question @Martinski4GitHub

It happens twice, once at around the 40-50% mark when loading after hitting save, and the other is when the loading is completed and the entire page refreshes.

[ExtremeFiretop]

@Martinski4GitHub

You'll also notice the directory bar never went red, which I think it's supposed too based on the code, right?

First one is here:

Second one is on the refresh:

[TheS1R]

Sorry guys, the girlfriend is calling for dinner. Will be back shortly.

To answer your question @Martinski4GitHub

It happens twice, once at around the 40-50% mark when loading after hitting save, and the other is when the loading is completed and the entire page refreshes.

And if I wait 4-5 seconds before hitting OK on first error dialogue, there is no second dialogue.

[Martinski4GitHub]

@Martinski4GitHub

You'll also notice the directory bar never went red, which I think it's supposed too based on the code, right?

First one is here:

Second one is on the refresh:

That's a completely different timing from my router. I get only one error dialog box at the end, after 100% is reached and the refresh happens, so the validation is called only once in my case. It's a very odd timing situation on your particular router.

The directory path text box does not go red likely because, by the time the refresh of the webGUI is completed, the previously correct entry is placed instead of the invalid entry so it goes to a "valid" state.

[Martinski4GitHub]

Sorry guys, the girlfriend is calling for dinner. Will be back shortly.
To answer your question @Martinski4GitHub
It happens twice, once at around the 40-50% mark when loading after hitting save, and the other is when the loading is completed and the entire page refreshes.

And if I wait 4-5 seconds before hitting OK on first error dialogue, there is no second dialogue.

Another interesting clue that certainty points to a timing issue!!

[Martinski4GitHub]

Just FYI,
Going to have dinner now and then "movie night" tonight!!!
Back in a couple of hours, if I don't get too tired and get sleepy after the movie, LOL!! :>).

[ExtremeFiretop]

@Martinski4GitHub

The problem goes away if I comment out this line:

setTimeout(GetExternalCheckResults,4000);

From the SaveAdvancedConfig function, and removed the call to GetExternalCheckResults() during the save window (the 10-second wait).

Another option is to set a global isFormSubmitting = false

And then modify the function GetExternalCheckResults to the below:

function GetExternalCheckResults()
{
	$.ajax({
		url: '/ext/MerlinAU/CheckHelper.js',
		dataType: 'script',
		timeout: 5000,
		error: function(xhr){
			// Only retry if we’re NOT in the middle of submitting
			if (!isFormSubmitting) {
				setTimeout(GetExternalCheckResults,1000);
			}
		},
		success: function()
		{
            // If we’re already in the middle of a submission, skip
            if (isFormSubmitting) {
                return;
            }

            if (externalCheckOK)
            {
               fwUpdateDirPath.extCheckOK = true;
               fwUpdateDirPath.extCheckMsg = '';
               return true;
            }
            if ((externalCheckID & fwUpdateDirPath.extCheckID) > 0)
            {
                externalCheckOK = true; //Reset for next check
                fwUpdateDirPath.extCheckOK = false;
                fwUpdateDirPath.extCheckMsg = externalCheckMsg;

                let fwUpdateDirectory = document.getElementById('fwUpdateDirectory');
                if (!ValidateDirectoryPath (fwUpdateDirectory))
                {
                    alert('Validation failed.\n\n' + fwUpdateDirPath.ErrorMsg());
                    return false;
                }
            }
		}
	});
}

[ExtremeFiretop]

@Martinski4GitHub

However the bug of the text box not going red still exists, it goes red if I click away from the browser while it's refreshing and then click back into the browser for the okay window.

But if I remain in the browser with it selected the entire time and go straight to the ok button, it does not go red.

[ExtremeFiretop]

@Martinski4GitHub

Removing the formField.focus(); from function ValidateDirectoryPath has resolved the red directory path issues.

By programmatically focusing on the element I think we are triggering any native or ASUSWRT-specific CSS for a focused input and that “in-focus” styling can override the .Invalid { background-color: red !important; } rule, can you try it and advise?

[ExtremeFiretop]

Removing all these:

formField.focus();

Has solved the red textbox issues when focused on the webpage.
With this currently, the box does not go red unless the browser is not in focus.

[ExtremeFiretop]

Removing this line:

setTimeout(GetExternalCheckResults,4000);

As solved the double "pop-up" issues.

[ExtremeFiretop]

Now I only get this pop-up:

(None with loading/refreshing)

And the box goes red after I click Ok:

[Martinski4GitHub]

@ExtremeFiretop,

Sorry, bud. After dinner, we finished watching the 3rd season of Jack Ryan on Prime Video, and then just went to sleep. I'll take a look at your proposed fixes tomorrow evening; I just got up to take a leak and saw your messages, but I'm too sleepy to think logically & straight at the moment :>)

Have a good night's sleep.

[TheS1R]

Now I only get this pop-up:

(None with loading/refreshing)

And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

[ExtremeFiretop]

Now I only get this pop-up:

(None with loading/refreshing)
And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

Yes. It turns red to identify that value was not correct and did not get saved due to validation.

That is how I see @Martinski4GitHub s code

[ExtremeFiretop]

Now I only get this pop-up:

(None with loading/refreshing)
And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

Or will not be saved* to be specific.

[TheS1R]

Now I only get this pop-up:

(None with loading/refreshing)
And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

Yes. It turns red to identify that value was not correct and did not get saved due to validation.

That is how I see @Martinski4GitHub s code

And does a browser refresh clear the red?

[ExtremeFiretop]

Now I only get this pop-up:

(None with loading/refreshing)
And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

Yes. It turns red to identify that value was not correct and did not get saved due to validation.
That is how I see @Martinski4GitHub s code

And does a browser refresh clear the red?

Yes it should

[ExtremeFiretop]

Now I only get this pop-up:

(None with loading/refreshing)
And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

Yes. It turns red to identify that value was not correct and did not get saved due to validation.
That is how I see @Martinski4GitHub s code

And does a browser refresh clear the red?

Actually I just tested and if I manually refresh the browser, the red remains. Unless I edit the box.
Maybe that's another bug to look into, I expected a refresh to clear the red. (Since the invalid value was not saved)

[Martinski4GitHub]

@Martinski4GitHub

The problem goes away if I comment out this line:

setTimeout(GetExternalCheckResults,4000);

From the SaveAdvancedConfig function, and removed the call to GetExternalCheckResults() during the save window (the 10-second wait).

Another option is to set a global isFormSubmitting = false

And then modify the function GetExternalCheckResults to the below:

		success: function()
		{
            // If we’re already in the middle of a submission, skip
            if (isFormSubmitting) {
                return;
            }

I like your 2nd option to use a global variable to indicate the status of the form submission. This gives the external validation function some control back despite the asynchronous nature of the WebGUI calls. I just submitted a new commit that includes this change.

I've tested it on my router and it still works, getting the error dialog box after 100% is reached and the refresh happens.

[Martinski4GitHub]

@Martinski4GitHub

However the bug of the text box not going red still exists, it goes red if I click away from the browser while it's refreshing and then click back into the browser for the okay window.

But if I remain in the browser with it selected the entire time and go straight to the ok button, it does not go red.

WRT the text box not going red after the external validation fails & the automatic refresh occurs; it's actually correct when the text box is "not red" because when the refresh is completed, the "bad" entry is reset to the previously valid entry so it goes to a valid state. As long as the error/alert dialog box pops up and informs the user that the previous entry was invalid, we're OK, IMO.

The alternative would be to put back the "bad" directory entry which will trigger the validation and the "red box" style, but I don't think it's worth doing that. The "bad" entry has been handled and the user has been informed of the invalid entry; that's good enough in my book (and we have still "other fish to fry" :>).

I believe this falls in the category of "Don't let perfect be the enemy of the good."

[TheS1R]

@Martinski4GitHub
The problem goes away if I comment out this line:
setTimeout(GetExternalCheckResults,4000);
From the SaveAdvancedConfig function, and removed the call to GetExternalCheckResults() during the save window (the 10-second wait).
Another option is to set a global isFormSubmitting = false
And then modify the function GetExternalCheckResults to the below:

		success: function()
		{
            // If we’re already in the middle of a submission, skip
            if (isFormSubmitting) {
                return;
            }

I like your 2nd option to use a global variable to indicate the status of the form submission. This gives the external validation function some control back despite the asynchronous nature of the WebGUI calls. I just submitted a new commit that includes this change.

I've tested it on my router and it still works, getting the error dialog box after 100% is reached and the refresh happens.

I'm seeing the same using GT-BE98 Pro with 3006.102.3 firmware. Looks great!

[Martinski4GitHub]

Now I only get this pop-up:

...

(None with loading/refreshing)

And the box goes red after I click Ok:
...

This should not happen and, IMO, it could be confusing & counterintuitive to users to see the previously valid entry shown now as INVALID (i.e. "red" style), which means they are now forced to click inside the text box to make it VALID again.

[TheS1R]

Now I only get this pop-up:
...
(None with loading/refreshing)
And the box goes red after I click Ok:
...

This should not happen and, IMO, it could be confusing & counterintuitive to users to see the previously valid entry shown now as INVALID (i.e. "red" style), which means they are now forced to click inside the text box to make it VALID again.

I agree. Your latest commit flashes red for a split second, and then shows the VALID state.

[Martinski4GitHub]

Now I only get this pop-up:

(None with loading/refreshing)
And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

Yes. It turns red to identify that value was not correct and did not get saved due to validation.

That is how I see @Martinski4GitHub s code

Actually, the text box should be red only if the current value is INVALID. Once the previously valid directory path is put back after the refresh is completed, the text box goes to a VALID state, so it's all good.

[Martinski4GitHub]

@Martinski4GitHub
The problem goes away if I comment out this line:
setTimeout(GetExternalCheckResults,4000);
From the SaveAdvancedConfig function, and removed the call to GetExternalCheckResults() during the save window (the 10-second wait).
Another option is to set a global isFormSubmitting = false
And then modify the function GetExternalCheckResults to the below:

		success: function()
		{
            // If we’re already in the middle of a submission, skip
            if (isFormSubmitting) {
                return;
            }

I like your 2nd option to use a global variable to indicate the status of the form submission. This gives the external validation function some control back despite the asynchronous nature of the WebGUI calls. I just submitted a new commit that includes this change.
I've tested it on my router and it still works, getting the error dialog box after 100% is reached and the refresh happens.

I'm seeing the same using GT-BE98 Pro with 3006.102.3 firmware. Looks great!

Thanks for the confirmation.

[Martinski4GitHub]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported. Afterwards the F12 menu started flooding with these:

BTW, are you still getting the errors reported on the browser console?
AFAICT, the errors are occurring in one of the native, built-in JS files found in your router model which probably are different version from mine so I don't see any errors at all.

[TheS1R]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported. Afterwards the F12 menu started flooding with these:

BTW, are you still getting the errors reported on the browser console? AFAICT, the errors are occurring in one of the native, built-in JS files found in your router model which probably are different version from mine so I don't see any errors at all.

Currently, no browser console errors.

[ExtremeFiretop]

@Martinski4GitHub
The problem goes away if I comment out this line:
setTimeout(GetExternalCheckResults,4000);
From the SaveAdvancedConfig function, and removed the call to GetExternalCheckResults() during the save window (the 10-second wait).
Another option is to set a global isFormSubmitting = false
And then modify the function GetExternalCheckResults to the below:

		success: function()
		{
            // If we’re already in the middle of a submission, skip
            if (isFormSubmitting) {
                return;
            }

I like your 2nd option to use a global variable to indicate the status of the form submission. This gives the external validation function some control back despite the asynchronous nature of the WebGUI calls. I just submitted a new commit that includes this change.

I've tested it on my router and it still works, getting the error dialog box after 100% is reached and the refresh happens.

Happy you liked the solution! :) Works well here.

[ExtremeFiretop]

@Martinski4GitHub
However the bug of the text box not going red still exists, it goes red if I click away from the browser while it's refreshing and then click back into the browser for the okay window.
But if I remain in the browser with it selected the entire time and go straight to the ok button, it does not go red.

WRT the text box not going red after the external validation fails & the automatic refresh occurs; it's actually correct when the text box is "not red" because when the refresh is completed, the "bad" entry is reset to the previously valid entry so it goes to a valid state. As long as the error/alert dialog box pops up and informs the user that the previous entry was invalid, we're OK, IMO.

The alternative would be to put back the "bad" directory entry which will trigger the validation and the "red box" style, but I don't think it's worth doing that. The "bad" entry has been handled and the user has been informed of the invalid entry; that's good enough in my book (and we have still "other fish to fry" :>).

I believe this falls in the category of "Don't let perfect be the enemy of the good."

I kinda liked that it went red for an invalid entry after clicking okay, as a way to "draw attention" to the box that needs adjustment.
(Lots of users don't even read and just the ok box because it's quick!)

The only issue I had with it, is I expected it to clear on a browser refresh, not on the box edit. Maybe that's just a difference in design view, I don't really care enough to mind either way :P

[ExtremeFiretop]

Now I only get this pop-up:
...
(None with loading/refreshing)
And the box goes red after I click Ok:
...

This should not happen and, IMO, it could be confusing & counterintuitive to users to see the previously valid entry shown now as INVALID (i.e. "red" style), which means they are now forced to click inside the text box to make it VALID again.

I guess it's only confusing because of the valid directory coming back in, if we went with the invalid directory again (as you mentioned) it wouldn't be confusing, it would actually help highlight what needs fixing (as mentioned above)

[ExtremeFiretop]

Now I only get this pop-up:
...
(None with loading/refreshing)
And the box goes red after I click Ok:
...

This should not happen and, IMO, it could be confusing & counterintuitive to users to see the previously valid entry shown now as INVALID (i.e. "red" style), which means they are now forced to click inside the text box to make it VALID again.

I agree. Your latest commit flashes red for a split second, and then shows the VALID state.

Agreed, that is what I get as well. (As long as I don't leave the browser window) if I leave focus then it still does this:

But I guess this is where Martinski's saying comes into play "Don't let perfection get in the way of good enough!" Hahaha!
This is for sure good enough. Merged!

[ExtremeFiretop]

Now I only get this pop-up:

(None with loading/refreshing)
And the box goes red after I click Ok:

Should it turn red when it is reverting to the previous value (i.e., before editing)?

Yes. It turns red to identify that value was not correct and did not get saved due to validation.
That is how I see @Martinski4GitHub s code

Actually, the text box should be red only if the current value is INVALID. Once the previously valid directory path is put back after the refresh is completed, the text box goes to a VALID state, so it's all good.

Gotcha

[ExtremeFiretop]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported. Afterwards the F12 menu started flooding with these:

BTW, are you still getting the errors reported on the browser console? AFAICT, the errors are occurring in one of the native, built-in JS files found in your router model which probably are different version from mine so I don't see any errors at all.

As clean as a whistle my friend, as clean as can be! How we like it!
(Unless dirty is something your looking for in code!)

[ExtremeFiretop]

More improvements & fine-tuning, primarily in the code that updates the configuration file from the settings changed via the WebGUI page. Checks are now made before updating the directory path of the F/W Update ZIP file to make sure it's a valid, existing directory. Also, some preliminary code to support informing the WebGUI of the failed validation of the directory path.

Love the cleanup you did in the shell script! The new Actions function is great, my original solution was just a quick and dirty method to get things up and running.

UPDATE: In my 2nd commit now, I added a simple validator in the WebGUI for the F/W Update directory path. After users click on the "Save" button, the shell script validates that the path actually exists and is writeable. The goal here is to make sure any path saved in the config file is indeed usable and ready to perform F/W updates.

Works as described, I did get the 2 prompts with an invalid directory similar to what @TheS1R reported. Afterwards the F12 menu started flooding with these:

BTW, are you still getting the errors reported on the browser console? AFAICT, the errors are occurring in one of the native, built-in JS files found in your router model which probably are different version from mine so I don't see any errors at all.

As clean as a whistle my friend, as clean as can be! How we like it! (Unless dirty is something your looking for in code!)

If it is; just come calling; I can help with dirty code. Poor Martinski, his skills are being tested!

[Martinski4GitHub]

Now I only get this pop-up:
...
(None with loading/refreshing)
And the box goes red after I click Ok:
...

This should not happen and, IMO, it could be confusing & counterintuitive to users to see the previously valid entry shown now as INVALID (i.e. "red" style), which means they are now forced to click inside the text box to make it VALID again.

I guess it's only confusing because of the valid directory coming back in, if we went with the invalid directory again (as you mentioned) it wouldn't be confusing, it would actually help highlight what needs fixing (as mentioned above)

Yeah, the alternative method of putting back the INVALID directory entry after the refresh so the text box goes "red" is certainly doable, but I'm not inclined to spend more time on this given that the current solution works and it's good enough for the add-on, IMO.

If some users don't read the error dialog box is beyond our control and not really our problem per se. Once the error alert has been shown, and users are required to click on a button to acknowledge the notification and continue, we have done our job. Anything beyond that is "extra icing on the cake."

If you want to take on that extra feature, I'm not really opposed to it either.

[ExtremeFiretop]

Now I only get this pop-up:
...
(None with loading/refreshing)
And the box goes red after I click Ok:
...

This should not happen and, IMO, it could be confusing & counterintuitive to users to see the previously valid entry shown now as INVALID (i.e. "red" style), which means they are now forced to click inside the text box to make it VALID again.

I guess it's only confusing because of the valid directory coming back in, if we went with the invalid directory again (as you mentioned) it wouldn't be confusing, it would actually help highlight what needs fixing (as mentioned above)

Yeah, the alternative method of putting back the INVALID directory entry after the refresh so the text box goes "red" is certainly doable, but I'm not inclined to spend more time on this given that the current solution works and it's good enough for the add-on, IMO.

If some users don't read the error dialog box is beyond our control and not really our problem per se. Once the error alert has been shown, and users are required to click on a button to acknowledge the notification and continue, we have done our job. Anything beyond that is "extra icing on the cake."

If you want to take on that extra feature, I'm not really opposed to it either.

Didn't you just react to my message above? It wouldn't be icing on the cake... It would more like... The raisins in a fruitcake if I take it on. 🤢

Hahaha I'm just kidding of course. Maybe I'll take a swing at it, but like you said it's work to rejig all the logic for little gain

[Martinski4GitHub]

Now I only get this pop-up:
...
(None with loading/refreshing)
And the box goes red after I click Ok:
...

This should not happen and, IMO, it could be confusing & counterintuitive to users to see the previously valid entry shown now as INVALID (i.e. "red" style), which means they are now forced to click inside the text box to make it VALID again.

I guess it's only confusing because of the valid directory coming back in, if we went with the invalid directory again (as you mentioned) it wouldn't be confusing, it would actually help highlight what needs fixing (as mentioned above)

Yeah, the alternative method of putting back the INVALID directory entry after the refresh so the text box goes "red" is certainly doable, but I'm not inclined to spend more time on this given that the current solution works and it's good enough for the add-on, IMO.
If some users don't read the error dialog box is beyond our control and not really our problem per se. Once the error alert has been shown, and users are required to click on a button to acknowledge the notification and continue, we have done our job. Anything beyond that is "extra icing on the cake."
If you want to take on that extra feature, I'm not really opposed to it either.

Didn't you just react to my message above? It wouldn't be icing on the cake... It would more like... The raisins in a fruitcake if I take it on. 🤢

Hahaha I'm just kidding of course. Maybe I'll take a swing at it, but like you said it's work to rejig all the logic for little gain

Yes, exactly. At work, that's one of the factors we keep in mind whenever any little feature or change is proposed:

Is the extra cost (i.e. time, money, resources, risk of regression errors) worth the gain (i.e. functionality/benefit to users), if other simpler alternatives exist?