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

Switched to using rundll32 with FileProtocolHandler to avoid flashing shell window #8

Merged
merged 2 commits into from
Feb 21, 2015

Conversation

oxtoacart
Copy link
Contributor

...ning UI

When using open-golang in a Windows program compiled with the linker flag -H=windowsgui, every time that open-golang is used, a command window temporarily appears.

This uses rundll32 with FileProtocolHandler to avoid that.

@oxtoacart oxtoacart changed the title Switched to using runhide to suppress command prompt on windows when ope... Switched to using runddl32 with FileProtocolHandler to avoid flashing shell window Feb 21, 2015
@oxtoacart oxtoacart changed the title Switched to using runddl32 with FileProtocolHandler to avoid flashing shell window Switched to using rundll32 with FileProtocolHandler to avoid flashing shell window Feb 21, 2015
@oxtoacart
Copy link
Contributor Author

@myleshorton Suggested using rundll32 which works great!

@skratchdot
Copy link
Owner

Thanks. I'll pull this in, but I'm going to re-add the call to the cleaninput function. I just tested from the command line in a windows VM:

loses "bar" variable:

C:\Windows\System32\run32dll.exe url.dll,FileProtocolHandler http://example.com/foo=1&bar=2

keeps "bar" variable:

C:\Windows\System32\run32dll.exe url.dll,FileProtocolHandler http://example.com/foo=1^&bar=2

Also, I'm not sure if there's a way to fix the openWith() function. I am also not sure if run32dll.exe exists on all versions of windows (i.e. XP-Win8+ and 32 vs 64 bit)- so hopefully this change doesn't introduce new issues for people (I seem to remember Win8 not having "start" in it's path, so the call to "cmd /C start" was needed).

skratchdot added a commit that referenced this pull request Feb 21, 2015
Switched to using rundll32 with FileProtocolHandler to avoid flashing shell window
@skratchdot skratchdot merged commit 6c04014 into skratchdot:master Feb 21, 2015
@oxtoacart
Copy link
Contributor Author

@skratchdot Thanks!

@myleshorton
Copy link

Yeah I think older windows versions may have issues recognizing paths
that don't end with .html as associated with the browser as their file
handler. We'll inevitably test this shortly though.

On Saturday, February 21, 2015, oxtoacart notifications@github.com wrote:

@skratchdot https://github.com/skratchdot Thanks!


Reply to this email directly or view it on GitHub
#8 (comment).

President
Brave New Software Project, Inc.
https://www.getlantern.org
pgp A998 2B6E EF1C 373E 723F A813 045D A255 901A FD89

@oxtoacart
Copy link
Contributor Author

It works on XP. I haven't tested anything older than that.

@myleshorton
Copy link

Nice!! Yeah I think it might just be Vista where things break, but that's
probably ok.

On Saturday, February 21, 2015, oxtoacart notifications@github.com wrote:

It works on XP. I haven't tested anything older than that.


Reply to this email directly or view it on GitHub
#8 (comment).

President
Brave New Software Project, Inc.
https://www.getlantern.org
pgp A998 2B6E EF1C 373E 723F A813 045D A255 901A FD89

@skratchdot
Copy link
Owner

Thanks for helping test/fix this guys. I'd like to get some automated tests setup with CI, but testing behavior like this is new to me (and I'm not sure of the best tool to do it). For instance, when manually testing in an XP vm, I can see that IE is opened to the correct page, but setting up an automated test might involve checking the process list before and after calling Open.run(), and then I'd only be sure that a browser was opened. I'd still need to do some type of screenshotting (and analysis) to confirm that the correct page was opened (or have something that could talk to a browser to see what URLs are open).

Anyways, maybe someday I'll try to work on that. For now I appreciate the manual testing you helped out with.

I don't think older than XP is any concern. I was more worried about newer versions of Windows, but we can deal with that if any issues are reported. Thanks again.

@myleshorton
Copy link

I tested on 8.1 originally, and it worked there.

On Saturday, February 21, 2015, ◬ notifications@github.com wrote:

Thanks for helping test/fix this guys. I'd like to get some automated
tests setup with CI, but testing behavior like this is new to me (and I'm
not sure of the best tool to do it). For instance, when manually testing in
an XP vm, I can see that IE is opened to the correct page, but setting up
an automated test might involve checking the process list before and after
calling Open.run(), and then I'd only be sure that a browser was opened.
I'd still need to do some type of screenshotting (and analysis) to confirm
that the correct page was opened (or have something that could talk to a
browser to see what URLs are open).

Anyways, maybe someday I'll try to work on that. For now I appreciate the
manual testing you helped out with.

I don't think older than XP is any concern. I was more worried about newer
versions of Windows, but we can deal with that if any issues are reported.
Thanks again.


Reply to this email directly or view it on GitHub
#8 (comment).

President
Brave New Software Project, Inc.
https://www.getlantern.org
pgp A998 2B6E EF1C 373E 723F A813 045D A255 901A FD89

@oxtoacart
Copy link
Contributor Author

@skratchdot Thanks for the handy library and the quick turnaround on this PR!

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

Successfully merging this pull request may close these issues.

None yet

4 participants