Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Javascript functions defined in an html menu are not registered on initial page load with Classic or Light skin only #817

Closed
SamInPgh opened this issue Sep 22, 2022 · 3 comments

Comments

@SamInPgh
Copy link
Contributor

This issue is discussed in the Material Skin forum beginning here. It only occurs when using the Classic or Light skin. JS functions are registered and available on the initial page load when using the Default skin. With Classic or Light, the page must be saved and reloaded in order for the functions to be recognized. This behavior can be observed in the player setup menu of the Denon AVP/AVR Control plugin starting with version 4.6.3, which will be released shortly.

@michaelherger
Copy link
Member

michaelherger commented Sep 23, 2022

Would the following change help?

diff --git a/HTML/EN/html/settings.js b/HTML/EN/html/settings.js
index 34788b1de..51ef2416a 100644
--- a/HTML/EN/html/settings.js
+++ b/HTML/EN/html/settings.js
@@ -32,23 +32,7 @@ function chooseSettings(value,option)
        }
 
        if (option) {
-               // change the mouse cursor so user gets some feedback
-               $('settingsForm').setStyle({cursor:'wait'});
-               new Ajax.Updater( { success: 'settingsRegion' }, url, {
-                       method: 'post',
-                       postBody: 'ajaxUpdate=1&player=[% playerURI %][% IF playerid %]&playerid=[% playerid | uri %][% END %]',
-                       evalScripts: true,
-                       asynchronous: true,
-                       onFailure: function(t) {
-                               alert('Error -- ' + t.responseText);
-                       },
-                       onComplete: function(t) {
-                               $('settingsForm').setStyle({cursor:'auto'});
-                               $('statusarea').update('');
-                               resizeSettingsSection();
-                       }
-               } );
-               //document.forms.settingsForm.action = url;
+               location.href = url + 'player=[% playerURI %][% IF playerid %]&playerid=[% playerid | uri %][% END %]';
        }
 }

I believe this is a nowadays optimisation to improve the situation from back in the days: as a full page load would be slow, that was not elegant enough. Therefore we tried to only update the inner parts of the page using an AJAX call. But these are limited and wouldn't allow to initialize JS in all cases. In fact, that settings UI in skins other than Default probably has been broken for years, if not a decade... but only with the popularity of Material this became obvious. The above change would no longer try to speed up page rendering by only loading partial content. Machines nowadays are fast enough to handle the redraw quickly. The change therefore would always reload the full page, initialising all JavaScript if needed.

@SamInPgh
Copy link
Contributor Author

SamInPgh commented Sep 24, 2022

I don't know what to say, Michael. How about "Wow!"? Great analysis and solution to one of those little outlier problems that we all would have gone on living with for another decade if not for your knowledge and problem solving abilities. The legend grows... Seriously, thank you for your prompt action on this. Now I don't have to update the plugin's wiki page with special instructions on using the setup menu with Material Skin . ;-)

P.S. I originally made the change before your edit and it did not go well. Thanks for staying with it and posting the update. I can't say that I understand much of it due to my relative ignorance of html but I am particularly confused about the use of the variable (keyword?) "uri" in the replacement statement, i.e. playerid=[% playerid | uri %]. It's not a typo for "url", is it? The bottom line is "it works", but I thought I'd ask anyway.

P.P.S. I just educated myself briefly on Universal Resource Identifiers and saw that this syntax is used throughout the LMS code. So basically I just proved my previous statement regarding my "relative ignorance of html" and maybe even qualify now for removing the word "relative" from it. Thanks again...

@michaelherger
Copy link
Member

Hehe... I wouldn't even have known the relation between uri and url by heart 😁. Had to look it up. But in the above case it refers to a function (filter in the Template Toolkit terms) which would "uri escape" data, make data save to be used as a url query parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants