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.

Sign up for GitHub

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

Jump to bottom

x/exp/shiny: windriver: client windows cannot be closed #25587

Closed
ktye opened this issue May 26, 2018 · 4 comments
Closed

x/exp/shiny: windriver: client windows cannot be closed #25587

ktye opened this issue May 26, 2018 · 4 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@ktye
Copy link

ktye commented May 26, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.9.2 windows/amd64

Does this issue reproduce with the latest release?

It should be unrelated

What operating system and processor architecture are you using (go env)?

windows/amd64

What did you do?

  • create a second window using shiny.
  • try to close the second window by clicking the x button in the window manager.

What did you expect to see?

the window to disappear

What did you see instead?

  • it is still there but unusable

Is it still the right place to create issues and to send patches for shiny?

The solution to the issue is:

  • DefWindowProc has to be called after WM_CLOSE is received.

The MS docs say:
An application can prompt the user for confirmation, prior to destroying a window, by processing the WM_CLOSE message and calling the DestroyWindow function only if the user confirms the choice.

By default, the DefWindowProc function calls the DestroyWindow function to destroy the window.

I could send in a CL if this is the right place.
The fix would be in:
x/exp/shiny/driver/internal/win32/win32.go: in func windowWndProc:

if uMsg == _WM_CLOSE {
fn(hwnd, uMsg, wParam, lParam) // This calls sendClose
return _DefWindowProc(hwnd, uMsg, wParam, lParam) // We have to run DefWindowProc after it and not just return
}

Of course this closes the window immediately without possible interception, which isn't supported by shiny at this stage anyway.
@gopherbot gopherbot added this to the Unreleased milestone May 26, 2018
@ktye ktye mentioned this issue May 26, 2018
Closed
@alexbrainman
Copy link
Member

@ktye can you provide some small program example that I can run to see the problem for myself?

Of course this closes the window immediately without possible interception, which isn't supported by shiny at this stage anyway.

We could put TODO in that code for the future.

Alex

@ktye
Copy link
Author

ktye commented May 27, 2018

This is a minimal example for you:
go get github.com/ktye/goissues/25587

A nicer one, would be:
github.com/mjl-/duit/examples/alert
after installing the shiny backend manually: github.com/ktye/duitdraw

If you accept the 3 line fix mentioned above and commit it yourself, I'm perfectly ok with it.
Otherwise I'll prepare a CL.

Kai

@alexbrainman
Copy link
Member

This is a minimal example for you:
go get github.com/ktye/goissues/25587

This is certainly broken. Thanks for taking time to create an example.

If you accept the 3 line fix mentioned above and commit it yourself, I'm perfectly ok with it.
Otherwise I'll prepare a CL.

Please, prepare the CL. It takes 2 people to make the change - first
person who sends the change, and second person who review the change.
I cannot be both. Let me know, if you need help creating the change.

The fix would be in:
x/exp/shiny/driver/internal/win32/win32.go: in func windowWndProc:

I disagree. I think you should add call into _DefWindowProc in
sendClose instead. All WM_CLOSE handling logic is in sendClose, so we
might as well add _DefWindowProc call there too.

Of course this closes the window immediately without possible interception, which isn't supported by shiny at this stage anyway.

I think you should also add a TODO in sendClose to say that WM_CLOSE
could be ignored once shiny has logic to support asking user if window
should be closed or not. My intention is to help future users to
understand our predicament.

/cc @nigeltao just FYI

Alex

@gopherbot
Copy link

Change https://golang.org/cl/115515 mentions this issue: x/exp/shiny: windriver: close client windows

@golang golang locked and limited conversation to collaborators May 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@gopherbot @alexbrainman @ktye