Option to Fully Disable / Detach state.js modifications

[ExtremeFiretop]

@Martinski4GitHub

Food for thought on this? You can follow the discussion here for the details and inspiration :
https://www.snbforums.com/threads/long-time-asus-merlin-first-time-amtm-reassurance-please.96827/post-986236

In short, the idea is to offset some of the bad press scMerlin has received over the years due to making modifications to state.js causing things to break the WebUI in firmware upgrades. This change means we no longer make it mandatory to have the drop down options, and make it user controllable and a separate feature detached of scMerlin.

If something breaks in a future firmware upgrade, a single click and the user is back in business as we unload the modifications to state.js

Might need further testing and put through it's paces with both 3006 and 3004 firmware.
I wouldn't be surprised if I introduced a bug or two as this isn't my regular project I'm familiar with, so please put it through the linter tool as well when you have a moment! :)

Thanks buddy! Hope you agree with the idea/premise

[ExtremeFiretop]

@Martinski4GitHub

After some reflection I'm thinking we should add a similar option for the sitemap since it also requires modifications to state.js.

Thoughts?

[Martinski4GitHub]

@Martinski4GitHub

Food for thought on this? You can follow the discussion here for the details and inspiration : https://www.snbforums.com/threads/long-time-asus-merlin-first-time-amtm-reassurance-please.96827/post-986236

In short, the idea is to offset some of the bad press scMerlin has received over the years due to making modifications to state.js causing things to break the WebUI in firmware upgrades. This change means we no longer make it mandatory to have the drop down options, and make it user controllable and a separate feature detached of scMerlin.

If something breaks in a future firmware upgrade, a single click and the user is back in business as we unload the modifications to state.js

I fully agree that it's a good idea to make the "WebUI left-side menu dropdowns" feature optional, but not necessarily as "a separate feature detached of scMerlin." Instead, it would be a new user-selectable option available within the scMerlin menu.

Might need further testing and put through it's paces with both 3006 and 3004 firmware. I wouldn't be surprised if I introduced a bug or two as this isn't my regular project I'm familiar with, so please put it through the linter tool as well when you have a moment! :)

I'll start to take a look later this evening, but probably will not finish the review until Friday or Saturday evening, depending on how things go the rest of the week (I have a work deadline coming up next week and some personal stuff to take care of this week).

We'll talk later.

[Martinski4GitHub]

@Martinski4GitHub

After some reflection I'm thinking we should add a similar option for the sitemap since it also requires modifications to state.js.

Thoughts?

Yes, you might as well make the Sitemap and the "Help and Support" WebUI pages also user-selectable options.

[ExtremeFiretop]

Hey @Martinski4GitHub buddy

I added code to also manage the unloading of the Sitemap.asp (as discussed)

However I left the Help and Support tab, since that seems to actually stem from the shared-jy and not from scMerlin, also the Help And Support does not require modifications to the state.js javascript file, so it doesn't have nearly as heavy of modifications/dependencies like the other scMerlin functions (sitemap and drop down menus)

I also tested this across both 3004 and 3006 firmware with expected behavior, seems to work correctly for me. However I can always use an extra pair of eyes!

I was not sure if you wanted it as 2 separate options (one for the drop downs and one for the sitemap), or a single option for both, so I opted to have a single option that unloads all the modifications instead of one for the drop down and one for the sitemap. However I made sure to keep the code for each in their own separate functions incase you do want separate buttons for each.

Let me know if you have any questions or notice anything while testing!
Joel S.

[ExtremeFiretop]

Now that we've handled unloading the modifications to state.js - I also wonder about any modifications to the index_style.css

Would this be a potential area of concern for firmware upgrades and breaking the webUI? Maybe?

Right now we continue to load modifications to the css at index_style.css , just wanted to bring that to your attention. (not trying to hide anything)

I believe the actual source of issues in the past was state.js , but the css could probably cause issues as well, I'm assuming.
So I guess the remaining question would be if it's worth restoring that to defaults as well when unloading changes to state.js or if the changes are low risk enough to ignore for now.

[Martinski4GitHub]

Hey @Martinski4GitHub buddy

I added code to also manage the unloading of the Sitemap.asp (as discussed)

Very good job, bud!!

However I left the Help and Support tab, since that seems to actually stem from the shared-jy and not from scMerlin, also the Help And Support does not require modifications to the state.js javascript file, so it doesn't have nearly as heavy of modifications/dependencies like the other scMerlin functions (sitemap and drop down menus)

Ah yes, good point, and it makes sense since that's a separate webpage with no ties to the state.js file.

I also tested this across both 3004 and 3006 firmware with expected behavior, seems to work correctly for me. However I can always use an extra pair of eyes!

I was not sure if you wanted it as 2 separate options (one for the drop downs and one for the sitemap), or a single option for both, so I opted to have a single option that unloads all the modifications instead of one for the drop down and one for the sitemap. However I made sure to keep the code for each in their own separate functions incase you do want separate buttons for each.

Either way (one option for both, or a separate option for each) is fine with me. I suspect the Sitemap page is not widely used, so having a single option for both should be fine for the vast majority of users.

Let me know if you have any questions or notice anything while testing! Joel S.

Apologies for the long "radio silence." My wife's mother had cataract surgery on Friday, so we've been over to their house all weekend to keep an eye on her recovery and take care of things. The 1st few days after surgery are crucial to avoid possible infection and inflammation risks, so my wife has been ensuring that the medications and post-surgery hygienic routines are followed religiously.

So I have had no time to do a full review of the PR changes, nor have I done proper testing. I'll try to do it whenever I can, but likely after I have completed my work deliverables due end-of-day on March 12th.

Talk to you later, bud.

[Martinski4GitHub]

Now that we've handled unloading the modifications to state.js - I also wonder about any modifications to the index_style.css

Would this be a potential area of concern for firmware upgrades and breaking the webUI? Maybe?

Right now we continue to load modifications to the css at index_style.css , just wanted to bring that to your attention. (not trying to hide anything)

I believe the actual source of issues in the past was state.js , but the css could probably cause issues as well, I'm assuming. So I guess the remaining question would be if it's worth restoring that to defaults as well when unloading changes to state.js or if the changes are low risk enough to ignore for now.

Frankly, I don't recall the modifications made in the index_style.css file. They were likely added to support the Sitemap webpage and/or the WebUI left-side menu dropdowns, and as such, they wouldn't be needed if those 2 are disabled. I don't think CSS changes by themselves would cause issues if nobody else is using them, but it's probably better to remove them and have a "cleaner" slate.

Have a good week, bud.

[ExtremeFiretop]

However I left the Help and Support tab, since that seems to actually stem from the shared-jy and not from scMerlin, also the Help And Support does not require modifications to the state.js javascript file, so it doesn't have nearly as heavy of modifications/dependencies like the other scMerlin functions (sitemap and drop down menus)

Ah yes, good point, and it makes sense since that's a separate webpage with no ties to the state.js file.

Just minor details, but wanted you to know the reason behind my thinking. Naturally, if we learn down the line that shared-jy is actually causing issues, we can re-evaluate then :)

Either way (one option for both, or a separate option for each) is fine with me. I suspect the Sitemap page is not widely used, so having a single option for both should be fine for the vast majority of users.

Yeah exactly what I figured, I did leave them as 2 functions (one for the drop downs, one for the sitemap) JUSTTT incase though ;)

But my thought was the same as yours, probably few people use the sitemap and the real concern was unloading the changes if/when it breaks some day in the future, so I renamed the option and just made it a "one big bang" approach, if scMerlin breaks something again in the future, the single button is all the user needs to restore functionality and undo "all" changes to the WebUI.

Apologies for the long "radio silence." My wife's mother had cataract surgery on Friday, so we've been over to their house all weekend to keep an eye on her recovery and take care of things. The 1st few days after surgery are crucial to avoid possible infection and inflammation risks, so my wife has been ensuring that the medications and post-surgery hygienic routines are followed religiously.

Oh wow! I hope she recovers quickly.

As you know my father "recently" went through that, and he ended up being one of the few individuals with complications from the surgery.

Luckily the doctors seem to have stabilized the infection/inflammation and his eye is under control these days, the doctors initially weren't so sure if he was going to have permanent vision loss, the remaining side effect now is some permanent "blurriness" to his vision, they aren't sure will ever be fixed with the new lens.

All that to say I totally understand the situation your in. Do take care of her where you can! My father needed us to drive him around to the eye doctor every other day, and help him with basic tasks while his eye was patched up and under observation.

So I have had no time to do a full review of the PR changes, nor have I done proper testing. I'll try to do it whenever I can, but likely after I have completed my work deliverables due end-of-day on March 12th.

No rush, you know how I work, I am just poking away at it when I have free time, the goal is to help where I can.
I'm not trying to push through any changes quickly when it comes to OSR.

But I did test these latest changes and all seems to work for me! I think it's ready for your stamp of approval (or disapproval if you or the linter tool finds anything!)

[ExtremeFiretop]

Frankly, I don't recall the modifications made in the index_style.css file. They were likely added to support the Sitemap webpage and/or the WebUI left-side menu dropdowns, and as such, they wouldn't be needed if those 2 are disabled. I don't think CSS changes by themselves would cause issues if nobody else is using them, but it's probably better to remove them and have a "cleaner" slate.

It was a like 4-5 lines... Found below:

.menu_Addons { background: url(ext/shared-jy/addons.png); background-size: contain; }
.dropdown-content {top: 0px; left: 185px; visibility: hidden; position: absolute; background-color: #3a4042; min-width: 165px; box-shadow: 0px 8px 16px 0px rgba(0,0,0,0.2); z-index: 1000;}
.dropdown-content a {padding: 6px 8px; text-decoration: none; display: block; height: 100%; min-height: 20px; max-height: 40px; font-weight: bold; text-shadow: 1px 1px 0px black; font-family: Verdana, MS UI Gothic, MS P Gothic, Microsoft Yahei UI, sans-serif; font-size: 12px; border: 1px solid #6B7071;}
.dropdown-content a:hover {background-color: #77a5c6;}
.dropdown:hover .dropdown-content {visibility: visible;}

The main additions that are scMerlin related are: .dropdown:hover and .dropdown-content
The .menu_Addons again seems to be related to shared-jy and is required to add the icon to the "Addons" drop menu.

Due to this, I made the changes for .menu_Addons conditional on if the add-ons drop menu exists in MenuTree.js
If it does exist, then it keeps it, otherwise it removes it.

The others for .dropdown:hover and .dropdown-content are removed and added with the new control option on disable and enable.

I really don't think those are high risk, but as you mentioned, the cleaner the better when it comes to avoiding future issues.

[Martinski4GitHub]

However I left the Help and Support tab, since that seems to actually stem from the shared-jy and not from scMerlin, also the Help And Support does not require modifications to the state.js javascript file, so it doesn't have nearly as heavy of modifications/dependencies like the other scMerlin functions (sitemap and drop down menus)

Ah yes, good point, and it makes sense since that's a separate webpage with no ties to the state.js file.

Just minor details, but wanted you to know the reason behind my thinking. Naturally, if we learn down the line that shared-jy is actually causing issues, we can re-evaluate then :)

Agreed.

Either way (one option for both, or a separate option for each) is fine with me. I suspect the Sitemap page is not widely used, so having a single option for both should be fine for the vast majority of users.

Yeah exactly what I figured, I did leave them as 2 functions (one for the drop downs, one for the sitemap) JUSTTT incase though ;)

But my thought was the same as yours, probably few people use the sitemap and the real concern was unloading the changes if/when it breaks some day in the future, so I renamed the option and just made it a "one big bang" approach, if scMerlin breaks something again in the future, the single button is all the user needs to restore functionality and undo "all" changes to the WebUI.

Sounds good.

Apologies for the long "radio silence." My wife's mother had cataract surgery on Friday, so we've been over to their house all weekend to keep an eye on her recovery and take care of things. The 1st few days after surgery are crucial to avoid possible infection and inflammation risks, so my wife has been ensuring that the medications and post-surgery hygienic routines are followed religiously.

Oh wow! I hope she recovers quickly.

As you know my father "recently" went through that, and he ended up being one of the few individuals with complications from the surgery.

Luckily the doctors seem to have stabilized the infection/inflammation and his eye is under control these days, the doctors initially weren't so sure if he was going to have permanent vision loss, the remaining side effect now is some permanent "blurriness" to his vision, they aren't sure will ever be fixed with the new lens.

All that to say I totally understand the situation your in. Do take care of her where you can! My father needed us to drive him around to the eye doctor every other day, and help him with basic tasks while his eye was patched up and under observation.

I hope your Dad has by now fully recovered from his eye surgery with very minimal to no side effects, and things are under control. So far, my mother-in-law is doing well in her recovery, and my wife especially has been on top of things, making sure her Mom takes her medications at the scheduled times and keeps her eye patch clean, etc. Today, we drove her to the clinic for a follow-up visit to check the progress of her recovery. My father might need the same surgery sometime this year if his cataracts become worse to the point that it impairs his daily activities, especially driving around town.

So I have had no time to do a full review of the PR changes, nor have I done proper testing. I'll try to do it whenever I can, but likely after I have completed my work deliverables due end-of-day on March 12th.

No rush, you know how I work, I am just poking away at it when I have free time, the goal is to help where I can. I'm not trying to push through any changes quickly when it comes to OSR.

But I did test these latest changes and all seems to work for me! I think it's ready for your stamp of approval (or disapproval if you or the linter tool finds anything!)

Sounds like you've already done your due diligence. I'll let you know when I complete my part, most likely by Friday evening, or sometime over the coming weekend, depending on how things go with work and personal stuff.

Have a good night and take care, bud.

[Martinski4GitHub]

Frankly, I don't recall the modifications made in the index_style.css file. They were likely added to support the Sitemap webpage and/or the WebUI left-side menu dropdowns, and as such, they wouldn't be needed if those 2 are disabled. I don't think CSS changes by themselves would cause issues if nobody else is using them, but it's probably better to remove them and have a "cleaner" slate.

It was a like 4-5 lines... Found below:

.menu_Addons { background: url(ext/shared-jy/addons.png); background-size: contain; }
.dropdown-content {top: 0px; left: 185px; visibility: hidden; position: absolute; background-color: #3a4042; min-width: 165px; box-shadow: 0px 8px 16px 0px rgba(0,0,0,0.2); z-index: 1000;}
.dropdown-content a {padding: 6px 8px; text-decoration: none; display: block; height: 100%; min-height: 20px; max-height: 40px; font-weight: bold; text-shadow: 1px 1px 0px black; font-family: Verdana, MS UI Gothic, MS P Gothic, Microsoft Yahei UI, sans-serif; font-size: 12px; border: 1px solid #6B7071;}
.dropdown-content a:hover {background-color: #77a5c6;}
.dropdown:hover .dropdown-content {visibility: visible;}

The main additions that are scMerlin related are: .dropdown:hover and .dropdown-content The .menu_Addons again seems to be related to shared-jy and is required to add the icon to the "Addons" drop menu.

Due to this, I made the changes for .menu_Addons conditional on if the add-ons drop menu exists in MenuTree.js If it does exist, then it keeps it, otherwise it removes it.

The others for .dropdown:hover and .dropdown-content are removed and added with the new control option on disable and enable.

I really don't think those are high risk, but as you mentioned, the cleaner the better when it comes to avoiding future issues.

Looks like you've taken care of everything that was inserted into the state.js file, and which could have become a potential issue in future F/W releases. Good job. bud!!

[ExtremeFiretop]

I noticed some errors today in the F12 dev console, after adding the restart to httpd, and it was due to the AJAX loops continuing to try and fetch data and getting html back while restarting the WebUI.

Added some gates to silence the errors. It wasn't something I checked initially after adding the restarts to httpd but I noticed it today while testing, simple fix was to just gate the AJAX loops when we knew we were going to disrupt the WebUI

[Martinski4GitHub]

I noticed some errors today in the F12 dev console, after adding the restart to httpd, and it was due to the AJAX loops continuing to try and fetch data and getting html back while restarting the WebUI.

Added some gates to silence the errors. It wasn't something I checked initially after adding the restarts to httpd but I noticed it today while testing, simple fix was to just gate the AJAX loops when we knew we were going to disrupt the WebUI

Great catch, bud. This morning, I reviewed the PR changes and then ran testing and validation. So far, it looks very good.

I also put the source code through the Linter, and there were only some minor warnings (about not following some of our formatting standards and coding style) - really nothing to worry about. But most importantly: no errors!!!

There are some changes/improvements I'd like to make, so I'll approve this PR and then submit my changes in a separate PR for your review and validation.

Great job, bud!!!

[Martinski4GitHub]

Looking good - Approved.