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

Fix modeless COM browser dialog #1895

Closed
wants to merge 3 commits into from

Conversation

CristiFati
Copy link
Contributor

@CristiFati CristiFati commented Jun 9, 2022

A follow-up for #1894.
Fixes [SO]: win32com.client combrowse.main() (Python Object Browser) is not responding Python 3.9.

As I mentioned I am neither a COM expert nor an MFC enthusiast, and this part of PyWin32 is new to me, although I've seen some unorthodox (to me) stuff here :).

The actual fix is the 3rd commit. It's the best compromise I found. After the crash fix, ShowWindow wasn't starting the dialog message loop, so calling RunModalLoop was required. But for some reason when dialog was closed with IDCANCEL (Esc or by clicking the X button), the dialog disappeared, but the loop didn't exit, so Ctrl + Break was required in the console in order to kill the process (this didn't happen for IDOK (Enter) - which worked fine).
That's why EndModalLoop is needed in OnCancel (even if [MS.Docs]: CDialog::OnCancel instructs to call DestroyWindow for modeless dialogs, that doesn't work (probably to PyWin32 extra wrapping layer)).
Overriding RunModalLoop would have been the elegant solution, but dialog.Dialog doesn't have it, so the do_modal wrapper was born (its naming style suggests that it doesn't come from MFC).

Modeless dialog works now (as seen in the image from the answer).

I don't know, maybe in the future, this functionality could be moved up to Dialog, but that's a task far from trivial, which would require extensive testing.

kxrob added a commit to kxrob/pywin32 that referenced this pull request Aug 11, 2022
.. after d10d559 (non-modal COM browser inside PythonWin IDE).

combrowse.py may be run / called from outside PythonWin
as a standalone GUI in a console (mhammond#1895).

Run the browser modeless when called from PythonWin tool menu
or when run as script inside PythonWin GUI.

Fix similar startup of tlbrowse.py: win32api.GetConsoleTitle()
meanwhile returns empty string when
no console.
kxrob added a commit to kxrob/pywin32 that referenced this pull request Aug 11, 2022
.. after d10d559 (non-modal COM browser inside PythonWin IDE).

combrowse.py may be run / called from outside PythonWin
as a standalone GUI in a console (mhammond#1895).

Run the browser modeless when called from PythonWin tool menu
or when run as script inside PythonWin GUI.

Fix similar startup of tlbrowse.py: win32api.GetConsoleTitle()
meanwhile returns empty string when
no console.
@kxrob
Copy link
Collaborator

kxrob commented Aug 11, 2022

I think the problem in that SO question just came from me making the browser default modal in d10d559. I was not aware (or forgot) that combrowse.py may be run / called from a shell outside PythonWin as a standalone mini GUI.
Fixed I hope in #1924 - with some auto switching like in tlbrowse.py.

The low-level RunModalLoop() in a do_modeless() seems like a contradiction - and leads to those artificial follow-up problems. DoModal() ist already the thing which does a consistent modal dialog operation including message loop and proper ending of the dialog etc. - even as standalone GUI as it seems. (The MFC virtuals OnOK OnCancel are not supposed to return a value I think - void.)
Modeless operation (without a new message loop) makes only sense inside another (MFC/win32) GUI app - PythonWin - with an outer / main message loop - so e.g. one can continue doing other interaction there while the COM browser is floating as a non-blocking window similar to the object browser.

mhammond pushed a commit that referenced this pull request Aug 13, 2022
* Make modal dialog the default again in combrowse.main()

.. after d10d559 (non-modal COM browser inside PythonWin IDE).

combrowse.py may be run / called from outside PythonWin
as a standalone GUI in a console (#1895).

Run the browser modeless when called from PythonWin tool menu
or when run as script inside PythonWin GUI.

Fix similar startup of tlbrowse.py: win32api.GetConsoleTitle()
meanwhile returns empty string when
no console.

* Make scintilla build again on github CI

* freeze setuptools<=63.2.0 for now:
XXX With setuptools 63.4.3 .. 64.0.0 the scintilla nmake
run won't compile. (setuptools.msvc._msvc14_get_vc_env()
won't get the VC\INCLUDE into os.environ.)
Let's see if this will be fixed again in higher versions
of setuptools.

* print the cl.exe full path and environ INCLUDE during
_build_scintilla() / nmake for future tracking

* use and fixup scintilla.mak instead of scintilla_vc6.mak
(which is outdated lacks some dependencies)
Copy link
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CristiFati any chance you can check if the problem still exists after #1924? There are some nice improvements here, but is seems like we might as well avoid do_modeless if we can.

@CristiFati
Copy link
Contributor Author

Thank you for feedbacks. I didn't explicitly stated it in the description, but do_modeless is a (lame) workaround (gainarie). Will check it, but give me some time. I will post another comment when done.

@CristiFati
Copy link
Contributor Author

I completed my tests. This is the current situation (build with latest changes):

  • Modal dialog (which is now the default) works

  • Modeless crashes (as expected since no fix was attempted in this area)

Now regarding the commits:

  1. Yes, CDialog::OnCancel, CDialog::OnOK return void. But browser.dynamic_browser (which extends Dialog) also have return super().OnOK() I thought to make them consistent (especially since in Python return; and return NULL; are equivalent), so I think it's fine

  2. I don't see anything wrong

  3. Although it fixes the crash, it should be probably left out as it's incorrect from design perspective. Maybe a proper fix will arise. Ore temporary disable the modeless behavior?

@kxrob
Copy link
Collaborator

kxrob commented Aug 26, 2022

  • Modeless crashes (as expected since no fix was attempted in this area)

I guess you are trying to run combrowse.main(modal=False) somehow from a console script / interactive, like in SO, without main message loop. That would naturally just flash and immediately close and exit again.
Modeless is for running within Pythonwin (or another GUI): from Menu/Tools/COMBrowser usually, or by >>> combrowse.main(modal=False) # alt. mdi=True from its interactive window - and then does not block the Pythonwin GUI and can be used parallel.

For curiosity I run it from a console script "pseudo-modeless" with minial non-MFC "main message loop" in a further MessageBox - works surprisingly:

#!py -3
import win32api
from win32com.client.combrowse import main
main(modal=False)
win32api.MessageBox(0, "waiting!")

@CristiFati
Copy link
Contributor Author

I finally got the modeless purpose. So setting it from a script (like in SO) is not supported. I think it's fine. As a note: changing mdi has no effect.

Yes! the message box trick works!

@mhammond
Copy link
Owner

mhammond commented Aug 28, 2022

win32api.MessageBox(0, "waiting!")

Heh - yeah, or a trivial win32gui.PumpWaitingMessages() loop would work. Sounds like we don't need anything here though.

@mhammond mhammond closed this Aug 28, 2022
@CristiFati CristiFati deleted the cfati_dev00 branch August 29, 2022 14:48
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.

3 participants