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

[macOS][bug] changing the wallpaper restarts the Dock, which is annoying #70

Closed
sg-s opened this issue Aug 4, 2016 · 21 comments
Closed

Comments

@sg-s
Copy link

sg-s commented Aug 4, 2016

problem is as described: when the wallpaper is reset, the Dock reloads, causing it to close and open, which is very distracting.

Surely there is a better way of changing the wallpaper than reloading the Dock/Finder?

@boramalper
Copy link
Owner

Found the cause! We explicitly restart the Dock to (I think?) make OS X refresh the image.

Since I have no idea about OS X, maybe the author of OS X support (@jcmiller11) might help us here. :)

@sg-s
Copy link
Author

sg-s commented Aug 5, 2016

this may be a bit of a overkill, but maybe use wallpaper?

it takes just one line to change the wallpaper, and does not need you to kill the Dock:

wallpaper path/to/file

@sg-s
Copy link
Author

sg-s commented Aug 5, 2016

is there more than one killall?

I tried replacing that section with:

    elif de == "mac":
        subprocess.call(["wallpaper " + file_path])
        # subprocess.call(["osascript", "-e", 'tell application "System Events"\n'
        #                  'set theDesktops to a reference to every desktop\n'
        #                  'repeat with aDesktop in theDesktops\n'
        #                  'set the picture of aDesktop to \"' + file_path + '"\nend repeat\nend tell'])
        # subprocess.call(["killall", "Dock"])

but the Dock still restarts. mysterious.

@boramalper
Copy link
Owner

is there more than one killall?

No. It's even more mysterious for me as someone who never used a mac. :)

I'll look for a Python module to set the wallpaper, or going to create one as a separate project. The current status seems like a bit of mess and that really bothers me. I thought about using appscript but it's abandoned. Making people install a node.js app for a 300 lines of Python script seems like an overkill.

Thank you so much! I'm keeping this open, in case someone finds a solution.

@sg-s
Copy link
Author

sg-s commented Aug 5, 2016

nevermind, fixed it:

subprocess.call(["wallpaper", file_path])

works. you need to have wallpaper installed.

@boramalper
Copy link
Owner

Glad to hear! I'll keep the issue open though, for the problem is still not resolved.

@hche608
Copy link

hche608 commented Aug 7, 2016

@sg-s @boramalper
wallpaper
It works, but the code only runs once, I have to use code like this to keep it to run every 10 mins. This does not make sense.

        subprocess.call(["osascript", "-e", 'tell application "System Events"\n'
                         'set theDesktops to a reference to every desktop\n'
                         'repeat with aDesktop in theDesktops\n'
                         'set the picture of aDesktop to \"' + file_path + '"\nend repeat\nend tell'])
        #subprocess.call(["killall", "Dock"])
        subprocess.call(["wallpaper", file_path])

this can execute can every 10 mins.

@boramalper
Copy link
Owner

Thanks, but I'm strongly against using wallpaper, maybe I can dig into their source and find how they do it. :)

@sg-s
Copy link
Author

sg-s commented Aug 8, 2016

@hche608

not sure why you're using the AppleScript in addition to wallpaper. To be clear, just this works:

subprocess.call(["wallpaper", file_path])

@sg-s
Copy link
Author

sg-s commented Aug 8, 2016

@boramalper agreed; the simpler the better!

@jcmiller11
Copy link
Contributor

jcmiller11 commented Aug 9, 2016

Hey guys, was on vacation when this was happening! So as I commented when I added that line:

jcmiller11@8fc3013

The killing of the dock is to stop a problem where OS X would cache the wallpaper in RAM and not check to see if the picture file had changed given that it was using the same filename each time. It was kind of a hacky solution but it works. A more elegant solution might be to make the filename of the latest downloaded image be unique each time, I believe that would work, but I didn't pursue that method at the time because I didn't want to mess with too much of the non-platform specific stuff just to get OS X support up and going.

@boramalper would you be opposed to using unique filenames in other platforms rather than just the filename-latest.png ?

Also note, if you take a quick glance at how wallpaper does it, it seems to depend on a compiled binary written in objective-c: https:/sindresorhus/macos-wallpaper which itself depends on Apple's Appkit. No clue whether this method also has the issue with multiple calls to the same filename not always updating.

Edit: a quick search seems to suggest that the method that wallpaper uses also suffers from the same issue: http://stackoverflow.com/questions/34607809/redraw-desktop-background-without-restarting-dock-in-os-x That stack overflow question seems to suggest that using unique filenames will work... unless the user is currently in a fullscreen application when the update happens... which seems to just be a bug with OS X that should ideally be filed with Apple.

@jcmiller11
Copy link
Contributor

It's been a long time since I touched all of this code, and it's all coming back to me solving this problem when I first added that killall line. We can add code to the main system to do something like timestamps for a unique filename to partially solve the problem of the picture update only working occasionally, but then we need to deal with a more complex cleanup system to remove them later... which only solves the problem if the user is not in full screen... or we can just kill the dock, which is also inconvenient... particularly if the user makes use of some window management systems in OS X like minimizing....

Kinda sucks but I couldn't find a perfect solution :/ so I just went with the scorched earth approach by just killing the process

@sg-s
Copy link
Author

sg-s commented Aug 10, 2016

how bad would it be to use unique filenames on macOS? you could even use random file names -- i agree it is clunky but that seems to be the only reasonable way

@jcmiller11
Copy link
Contributor

would have to add in some whole way to store the name of the last file to delete, destroying the simplicity of relying on each native filesystem to overwrite the file, would most easily be implemented outside of the environment specific code, still wouldn't totally solve the problem. I'll wait for @boramalper to weigh in on this

@boramalper
Copy link
Owner

I think we can append the date in the file name. Then we'll delete the last image, and save the new one.

Since himawaripy is using appdirs to save images in its own dedicated directory, we can check if there is only one .png file with the prefix himawaripy- in that directory, we can delete that image and save the latest one; if not, we will exit the program to prevent any accidental data loss.

I'm really busy this week, but I'll try to implement it as soon as possible afterwards. :) Thank you all for your contribution!

@sg-s
Copy link
Author

sg-s commented Aug 10, 2016

how about a cruder approach?

  1. delete all .png files with the prefix "himawaripy" in the target folder
  2. download the new image, save it with a random name (or timestamp)
  3. set wallpaper (since filenames are now unique, it should work)

repeat

@boramalper
Copy link
Owner

@sg-s Why cruder? The only difference (checking whether there are more than one .png file to be deleted) is very very simple to implement (just checking the length of the results) yet can save us from a possible catastrophe. Given that how many things went wrong with this intended-to-be-small-script, I'm really hesitant of crude approaches. :)

@jcmiller11
Copy link
Contributor

Agreed, any script that deletes a file should do so very judiciously as a small bug can be catastrophic, for example: MrMEEE/bumblebee-Old-and-abbandoned#123

Using a random name might necessitate an extra check for the very small chance of a collision lest one in a million updates fail to display for mac users, lol. I think @boramalper 's proposed solution is the best choice.

@boramalper
Copy link
Owner

This commit should resolve the issue. Can you confirm @sg-s?

@sg-s
Copy link
Author

sg-s commented Aug 18, 2016

HI @boramalper, thanks! I think you forgot to remove this line:

subprocess.call(["killall", "Dock"])

in utils.py

I removed it, and it looks like it's working! Awesome!

@boramalper
Copy link
Owner

@sg-s Fixed. Thank you all so much!

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