OpenHere: Replace explorer window lookup code w/ site lookup #14048
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
When we first introduced the shell extension, it didn't work properly for some folders (such as the Desktop, or perhaps any "background" click) due to a bug in Windows. We worked around that bug with the help of an awesome community member, who contributed code that would pull up the topmost Explorer window and query its location. That Windows bug was eventually fixed, but we still had trouble with items appearing correctly. On Windows 11, the Open in Terminal menu item appears and disappears at random when you right-click the desktop, but it always appears when you right-click a folder. It sometimes appears for Quick Access, even though it shouldn't. We tried to fix that in #13206, but the fix caused more issues than it solved. We reverted it for 1.15 and 1.16. At the end of the day, it turns out that getting the path from the toplevel explorer window is fragile. Fortunately, the shell does offer us a way to get that information: the site chain. This pull request replaces GetPathFromExplorer() with an implementation of `IObjectWithSite`, which allows us to use the site chain to look up from whence a context menu request was initiated. It also makes item lookup generally more robust. ✅ Tested on Windows 11 ✅ Desktop ✅ Folder Background ✅ Folder Selected ✅ Quick Access (does not appear) ✅ This PC (does not appear) References #13206 References #13523 Closes #12578
ghost
added
Area-ShellExtension
|
/cc @leejy12 :) thanks for looking at this with us! I'd appreciate your review and signoff, if you can, since you're the most familiar with this code. |
|
I have to test this on Windows 10 19041! |
DHowett
changed the title
luedersj
approved these changes
Sep 21, 2022
lhecker
approved these changes
Sep 21, 2022
lhecker
approved these changes
Sep 21, 2022
DHowett
changed the title
DHowett
added a commit
that referenced
this pull request
Sep 21, 2022
When we first introduced the shell extension, it didn't work properly for some folders (such as the Desktop, or perhaps any "background" click) due to a bug in Windows. We worked around that bug with the help of an awesome community member, who contributed code that would pull up the topmost Explorer window and query its location. That Windows bug was eventually fixed, but we still had trouble with items appearing correctly. On Windows 11, the Open in Terminal menu item appears and disappears at random when you right-click the desktop, but it always appears when you right-click a folder. It sometimes appears for Quick Access, even though it shouldn't. We tried to fix that in #13206, but the fix caused more issues than it solved. We reverted it for 1.15 and 1.16. At the end of the day, it turns out that getting the path from the toplevel explorer window is fragile. Fortunately, the shell does offer us a way to get that information: the site chain. This pull request replaces GetPathFromExplorer() with an implementation of `IObjectWithSite`, which allows us to use the site chain to look up from whence a context menu request was initiated. It also makes item lookup generally more robust. * ✅ Tested on Windows 11 * ✅ Desktop * ✅ Folder Background * ✅ Folder Selected * ✅ Quick Access (does not appear) * ✅ This PC (does not appear) * ✅ Tested on Windows 10 * ✅ Desktop * ✅ Folder Background * ✅ Folder Selected * ✅ Quick Access (does not appear) * ✅ This PC (does not appear) References #13206 References #13523 Closes #12578 Co-authored-by: John Lueders <johnlue@microsoft.com> (cherry picked from commit 5027c80) Service-Card-Id: 85788409 Service-Version: 1.16
|
🎉 Handy links: |
ghost
mentioned this pull request
DHowett
added a commit
that referenced
this pull request
Sep 27, 2022
When we first introduced the shell extension, it didn't work properly for some folders (such as the Desktop, or perhaps any "background" click) due to a bug in Windows. We worked around that bug with the help of an awesome community member, who contributed code that would pull up the topmost Explorer window and query its location. That Windows bug was eventually fixed, but we still had trouble with items appearing correctly. On Windows 11, the Open in Terminal menu item appears and disappears at random when you right-click the desktop, but it always appears when you right-click a folder. It sometimes appears for Quick Access, even though it shouldn't. We tried to fix that in #13206, but the fix caused more issues than it solved. We reverted it for 1.15 and 1.16. At the end of the day, it turns out that getting the path from the toplevel explorer window is fragile. Fortunately, the shell does offer us a way to get that information: the site chain. This pull request replaces GetPathFromExplorer() with an implementation of `IObjectWithSite`, which allows us to use the site chain to look up from whence a context menu request was initiated. It also makes item lookup generally more robust. * ✅ Tested on Windows 11 * ✅ Desktop * ✅ Folder Background * ✅ Folder Selected * ✅ Quick Access (does not appear) * ✅ This PC (does not appear) * ✅ Tested on Windows 10 * ✅ Desktop * ✅ Folder Background * ✅ Folder Selected * ✅ Quick Access (does not appear) * ✅ This PC (does not appear) References #13206 References #13523 Closes #12578 Co-authored-by: John Lueders <johnlue@microsoft.com> (cherry picked from commit 5027c80) Service-Card-Id: 85788408 Service-Version: 1.15
|
🎉 Handy links: |
This was referenced Nov 22, 2022
|
🎉 Handy links: |
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.
When we first introduced the shell extension, it didn't work properly
for some folders (such as the Desktop, or perhaps any "background"
click) due to a bug in Windows. We worked around that bug with the help
of an awesome community member, who contributed code that would pull up
the topmost Explorer window and query its location.
That Windows bug was eventually fixed, but we still had trouble with
items appearing correctly. On Windows 11, the Open in Terminal menu item
appears and disappears at random when you right-click the desktop, but
it always appears when you right-click a folder. It sometimes appears
for Quick Access, even though it shouldn't.
We tried to fix that in #13206, but the fix caused more issues than it
solved. We reverted it for 1.15 and 1.16.
At the end of the day, it turns out that getting the path from the
toplevel explorer window is fragile. Fortunately, the shell does offer
us a way to get that information: the site chain.
This pull request replaces GetPathFromExplorer() with an implementation
of
IObjectWithSite, which allows us to use the site chain to look upfrom whence a context menu request was initiated. It also makes item
lookup generally more robust.
References #13206
References #13523
Closes #12578
Co-authored-by: John Lueders johnlue@microsoft.com