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

Rescan buttons "Classic" skin #410

Closed
kidstypike opened this issue Aug 31, 2020 · 37 comments
Closed

Rescan buttons "Classic" skin #410

kidstypike opened this issue Aug 31, 2020 · 37 comments

Comments

@kidstypike
Copy link

kidstypike commented Aug 31, 2020

Rescan buttons alongside individual media folders in Classic skin do nothing, which affects the "Material" skin.

@michaelherger
Copy link
Member

Working fine here - with one important limitation: once you've clicked one of those buttons you'd have to refresh the page once the scan has finished, as the buttons will be disabled while a scan is running. Can you double-check this behaviour?

@kidstypike
Copy link
Author

Just tried again using the Classic skin (not Default), button does nothing, no scan initiated.
rescanClassic

@kidstypike
Copy link
Author

Sorry, didn't mean to close the issue.

@michaelherger
Copy link
Member

I tested this again, and both Classic as well as Material do trigger a scan when I hit one of those buttons. What might be confusing, and I mentioned this before, is that once you've clicked one of those buttons, the other buttons would indeed become inactive, without showing any sign of this limitation. You'd have, after the scan has finished, reload the page to get the buttons back.

In the Default skin there's a poll going on in the back ground which would re-enable the buttons once the scan is done. I might want to re-create this for the old skins.

@kidstypike
Copy link
Author

Michael, with respect, If I add an album to say the first media folder in the list (/mnt/musicbox5/Squeezebox music) and click the rescan button alongside, nothing happens, no scan is initiated, I'm not attempting to click any of the other rescan buttons.

Works fine in Default skin.

Don't know what else to say.

@mherger
Copy link
Contributor

mherger commented Sep 2, 2020

Please enable info logging for control.command, try again, share your log files (both server and scanner).

@kidstypike
Copy link
Author

Logs attached,

server.log.zip
scanner.log.zip

@mherger
Copy link
Contributor

mherger commented Sep 10, 2020

I do indeed not see a correlation between the scanner.log's time stamps and traces in server.log. What browser are you using?

@kidstypike
Copy link
Author

Well exactly, the scanner log is from a previous scan initiated by me from the "default" skin a couple of days earlier. As I said the scan buttons in the classic skin do nothing, so nothing in the scanner log.

@kidstypike
Copy link
Author

Just tried this using:

Google Chrome
Firefox
Brave
Opera
Microsoft Edge

none work using classic skin.

@mherger
Copy link
Contributor

mherger commented Sep 11, 2020

Could you please share a screenshot of your basic settings? Or the server.prefs file?

@kidstypike
Copy link
Author

Basic settings

server.zip

@michaelherger
Copy link
Member

Trying to reproduce your issue brought up other bugs, but I still have no clue why that would not work. And you seem to be the only one suffering from this... would you have another machine where you could run a test whether this was a problem with that installation?

@kidstypike
Copy link
Author

Perhaps there's not many regulars use more than one media folder and try to scan just one of them using the Material or Classic skin?

Anyway, I've just tried this with exactly the same five media folders (on a different drive) on Win10 64 Pro/LMS 7.9.3 August 23rd. With the same result, clicking one of the "Rescan" buttons using the Classic skin does nothing except flash the screen for a millisecond, but works fine using the Default skin.

@mherger
Copy link
Contributor

mherger commented Feb 23, 2021

I looked into this code again. Unfortunately there's no useful logging there. Would you be able to edit a file or two to add some log statements?

@kidstypike
Copy link
Author

I'll give it a go, don't expect great things though.

@michaelherger
Copy link
Member

Ok, could you please add the following two warn statements, then restart LMS and click one of those rescan buttons. What do you get in server.log?

diff --git a/Slim/Web/Settings/Server/Basic.pm b/Slim/Web/Settings/Server/Basic.pm
index f07f6bc74..d76cb15c5 100644
--- a/Slim/Web/Settings/Server/Basic.pm
+++ b/Slim/Web/Settings/Server/Basic.pm
@@ -89,6 +89,7 @@ sub handler {
 		my $singleDirScan;
 		for (my $i = 0; defined $paramRef->{"pref_mediadirs$i"}; $i++) {
 			if (my $path = $paramRef->{"pref_mediadirs$i"}) {
+				warn Data::Dump::dump($oldPaths{$path}, $path);
 				delete $oldPaths{$path};
 				push @paths, $path;
 
@@ -108,6 +109,7 @@ sub handler {
 
 		my $oldCount = scalar @{ $prefs->get('mediadirs') || [] };
 
+		warn Data::Dump::dump(\%oldPaths, \@paths, $singleDirScan);
 		if ( keys %oldPaths || !$oldCount || scalar @paths != $oldCount ) {
 			$prefs->set('mediadirs', \@paths);
 		}

@kidstypike
Copy link
Author

As I don't have a clue where or how to add this, I think it's best if we stop right here.

Thanks.

@michaelherger
Copy link
Member

Interesting finding found by garym:

I get the exact same results as you (LMS 8.1.2 running on rPi4B 7.01). First "rescan" button does nothing (and reports no error or anything, just nothing happens). the 2nd one does rescan as expected.

Still can't reproduce it, though...

@kidstypike
Copy link
Author

I'm sorry I cannot assist further here.

@mherger
Copy link
Contributor

mherger commented Jul 25, 2022

@Apesbrain - please try what I outlined in #410 (comment)

As you're on Windows this might actually be impossible 😞. Would you mind sharing your server.log file with me?

@Apesbrain
Copy link

Apesbrain commented Jul 25, 2022

Would you mind sharing your server.log file with me?

No problem, but when I click on the suspect button there is nothing generated in the log. Can you tell me which logging parameters to enable?

I see the instruction above regarding "warn" statements, but do not know how to add this code.

@michaelherger
Copy link
Member

I'm sorry, I wanted to ask about server.prefs, rather than server.log...

@Apesbrain
Copy link

Ok, will send to you via Forum. Thanks.

@michaelherger
Copy link
Member

Thanks. Nothing obviously wrong in that file (except for an insane number of player configurations 😁).

Are you familiar with the browsers' dev tools? Could you please, right after loading the basic settings page, right click the button to see whether it's flagged disabled? Can you even click that button? If so: can you check the "Network" or similar tab in the dev tools to see what parameters are being sent to the server when you hit that button?

@Apesbrain
Copy link

Hope these attachments are helpful.

Clipboard01
Clipboard02

@michaelherger
Copy link
Member

Unfortunately in the second screenshot the important part is hidden behind the console tab. Please close it (cross on the right), and make sure you highlight the request to basic.html which sent a POST request:

POSTrequest

Then change to the Headers tab:
headers

@Apesbrain
Copy link

Hope this is it:

Clipboard01

@michaelherger
Copy link
Member

Ok, I added some logging to this handler. Please get the next build (due out in a bit), then enable INFO logging for scan.scanner. Hit the button and look for Path information for single dir scan in server.log.

@Apesbrain
Copy link

Apesbrain commented Jul 26, 2022

Just installed build 8.3.0 - 1658847186 @ Tue Jul 26 17:16:50 WEDT 2022 and followed your instructions. No such line appears in server.log. Wrong build?

@michaelherger
Copy link
Member

I'm missing the saveSettings parameter in your last screenshot. Is it really not there, or is it hidden at the bottom of the params list?

@Apesbrain
Copy link

It's not there.

@michaelherger
Copy link
Member

Ok, that would explain why the scan isn't triggered. Now the question is: why is it not there... I've double checked with Safari, Chrome and Firefox, and it's working with all of them here.

Are you using any add-ons in your browsers which might have an impact on the javascript or form submission behavior? Any non-standard setting you're using?

Looking at the page's code I see that we added some "hack" for Safari specifically to work around an issue which sounds similar for Safari only - 14 years ago! The message reads (with all my typos 😬):

add hidden "saveSettins" form element for Safari, as it doesn't send the submit button's value when submitted using JS.

I wonder whether that's a behavior Safari introduced a long time ago, and others are following optionally. I might just enable that hack for all browsers.

@michaelherger
Copy link
Member

michaelherger commented Jul 27, 2022

Now I feel really stupid... as I fired up all browsers again to check whether they would still work with the above suggested change I'm suddenly seeing the reported behavior... I must have tested this a dozen times, even only just installed Firefox to run this test - and did not see the problem before (see my screenshots from Chrome, where saveSettings is prominently at the top). Oh well... the workaround does its job for all of them. Triple checked going back and forth between modified and original code.

@kidstypike
Copy link
Author

kidstypike commented Jul 27, 2022 via email

@Apesbrain
Copy link

Thanks for persevering on this one!

@mherger
Copy link
Contributor

mherger commented Oct 11, 2022 via email

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

No branches or pull requests

4 participants