fix: don't set delegate for QLPreviewPanel
#37530
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeremyspiegel
commented
Mar 8, 2023
| }); | ||
|
|
||
| w.previewFile(__filename); | ||
| await setTimeout(500); |
There was a problem hiding this comment.
I couldn't figure out a great way to test this without adding another internal event just for testing. I also tried await waitUntil(() => !w.isFocused()); but that didn't wait long enough for the second show.
I would have also liked to create a test with another window covering the QuickLook window but that doesn't work either.
codebytere
changed the title
QLPreviewPanel
codebytere
added
semver/patch
codebytere
approved these changes
Mar 13, 2023
|
Release Notes Persisted
|
This was referenced Mar 14, 2023
|
I have automatically backported this PR to "22-x-y", please check out #37576 |
|
I have automatically backported this PR to "23-x-y", please check out #37577 |
trop
bot
removed
the
target/22-x-y
|
I have automatically backported this PR to "24-x-y", please check out #37578 |
trop
bot
added
in-flight/23-x-y
and removed
target/23-x-y
Closed
trop
bot
added
merged/23-x-y
trop
bot
added
merged/24-x-y
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Fixes #37476
The QuickLook NSWindow's delegate is set to the same object as the delegate for the BrowserWindow's NSWindow here. The delegate's handler for windowDidChangeOcclusionState doesn't seem to expect handling events for the QuickLook window, and so when the QuickLook window becomes occluded, it assumes that the BrowserWindow has been occluded.
There's a similar issue for other NSWindowDelegate methods besides windowDidChangeOcclusionState. So resizing/moving the QuickLook window will cause the BrowserWindow's resize/will-resize/resized/move/will-move/moved events to fire.
The code has moved around a bit but I think this may have existed since BrowserWindow.previewFile was added in 2016 in #7592. FYI @lauracpierre @zcbenz @bengotow
Checklist
npm testpassesRelease Notes
Notes: Fixed issue with BrowserWindow not updating after call to previewFile.