Skip to content

bpo-37929: avoid Squeezer-related config dialog crashes #15452

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

Merged

Conversation

taleinat
Copy link
Contributor

@taleinat taleinat commented Aug 24, 2019

These crashes were caused by keeping around a reference to the Squeezer instance and calling it's load_font() upon config changes, which sometimes happened even if the shell window no longer existed.

This change completely removes that update mechanism, instead having the editor window properly update its width attribute, which can then be used by Squeezer.

Note that the editor window's width is already used in a couple of other places, where it can currently be the incorrect value if the font, font size and/or window width are changed. With this PR, the entire codebase can correctly assume that EditorWindow.width is the updated width of the text widget in characters.

https://bugs.python.org/issue37929

These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

Looks good to me. I manually tested code-context and squeezer for some possible regressions.
Please merge Sunday if you think ready, so in b4.

@taleinat taleinat merged commit d4b4c00 into python:master Aug 25, 2019
@taleinat taleinat deleted the bpo-37929/fix-configdialog-squeezer-crash branch August 25, 2019 05:53
@miss-islington
Copy link
Contributor

Thanks @taleinat for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @taleinat, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker d4b4c00b57d24f6ee2cf3a96213406bb09953df3 3.8

@bedevere-bot
Copy link

GH-15484 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 25, 2019
…GH-15452)

These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
(cherry picked from commit d4b4c00)

Co-authored-by: Tal Einat <[email protected]>
@miss-islington
Copy link
Contributor

Thanks @taleinat for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-15485 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 25, 2019
…GH-15452)

These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
(cherry picked from commit d4b4c00)

Co-authored-by: Tal Einat <[email protected]>
@taleinat
Copy link
Contributor Author

Looks good to me. I manually tested code-context and squeezer for some possible regressions.
Please merge Sunday if you think ready, so in b4.

Thanks for the quick review @terryjreedy! Merged.

miss-islington added a commit that referenced this pull request Aug 25, 2019
These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
(cherry picked from commit d4b4c00)

Co-authored-by: Tal Einat <[email protected]>
miss-islington added a commit that referenced this pull request Aug 25, 2019
These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
(cherry picked from commit d4b4c00)

Co-authored-by: Tal Einat <[email protected]>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
…GH-15452)

These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
…GH-15452)

These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
…GH-15452)

These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants