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 ownership of FormUpdates #8861

Merged
merged 1 commit into from Feb 17, 2021

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Feb 17, 2021

FormBrowse is the main form, and hence it doesn't have Owner set. This leads to FormUpdates being created as an orphan modal form, that can be hidden behind FormBrowse and making the app look unresponsive.

image

Correct the ownership and, thus, force the new form to show above the main form.

Test methodology

  • manual

✒️ I contribute this code under The Developer Certificate of Origin.

`FormBrowse` is the main form, and hence it doesn't have `Owner` set.
This leads to `FormUpdates` being created as an orphant modal form,
that can be hidden behind `FormBrowse` and making the app look
unresponsive.

Correct the ownership and, thus, force the new form to show above
the main form.
@ghost ghost assigned RussKie Feb 17, 2021
@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #8861 (1a635a6) into master (e40b0fc) will decrease coverage by 2.97%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #8861      +/-   ##
==========================================
- Coverage   56.11%   53.13%   -2.98%     
==========================================
  Files         919      844      -75     
  Lines       65523    60927    -4596     
  Branches    11997    11672     -325     
==========================================
- Hits        36768    32375    -4393     
+ Misses      25758    25695      -63     
+ Partials     2997     2857     -140     
Flag Coverage Δ
production 43.33% <0.00%> (+0.06%) ⬆️
tests 94.39% <ø> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ghost
Copy link

ghost commented Feb 17, 2021

Maybe just use null? 'FormBrowse' is always owner.

@RussKie
Copy link
Member Author

RussKie commented Feb 17, 2021

null leads to the problem I'm fixing. All dialogs we open from FormBrowse within the same process must have correct owner (i.e. non null) set to prevent child windows getting lost behind and locking the app.

@RussKie RussKie merged commit 26361af into gitextensions:master Feb 17, 2021
@RussKie RussKie deleted the fix_FormUpdates_ownership branch February 17, 2021 13:02
@ghost ghost added this to the 3.6 milestone Feb 17, 2021
@RussKie RussKie modified the milestones: 3.6, 3.5 Feb 17, 2021
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

2 participants