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

Windows Terminal not 100% shown in context menu #13523

Closed
lychichem opened this issue Jul 17, 2022 · 10 comments
Closed

Windows Terminal not 100% shown in context menu #13523

lychichem opened this issue Jul 17, 2022 · 10 comments
Assignees
Labels
Area-ShellExtension For issues related to the explorer right-click context menu Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.

Comments

@lychichem
Copy link

Windows Terminal version

1.14.1861.0

Windows build number

10.0.22622.290

Other Software

No response

Steps to reproduce

  1. Install windows terminal 1.14.1861.0
  2. Right click in some folder
  3. The bug happens.

Expected Behavior

The "open in terminal" should display every time.

Actual Behavior

The "open in terminal" don't display every time. I can see it 1 time when I right click every around 6 times.

@lychichem lychichem added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Jul 17, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jul 17, 2022
@lychichem lychichem reopened this Jul 17, 2022
@lychichem
Copy link
Author

Maybe related to powertoys as it happens just after powertoys updated to 0.60.x

@zadjii-msft
Copy link
Member

Hmm, that sure is weird. Can you file feedback using the Feedback Hub, under the "Desktop Environment > Right Click Context Menu" area/?

image

I'm not sure there's much we can do to investigate here, but getting this to the team that owns the context menus might help the investigation. If you "Share" the aka.ms link to that feedback here, I can make sure to follow the internal bug that gets generated. Thanks!

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 18, 2022
@lychichem
Copy link
Author

lychichem commented Jul 18, 2022

Now I uninstalled powertoys, things looks ok. Will test again by reinstall powertoys.
Edit: confirmed this related to powertoys 0.60.x. Submitted to windows feedback with same title and content. And I guess it relates to this highlight:
image

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 18, 2022
@zadjii-msft zadjii-msft added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Jul 18, 2022
@Skywings96
Copy link

Skywings96 commented Jul 19, 2022

I am very sure this problem is related to the latest version powertoys. If you want to fix it, you just need turn off Power Rename and Image Resizer.

@lychichem
Copy link
Author

I am very sure this problem is related to the latest version powertoys. If you want to fix it, you just need turn off Power Rename and Image Resizer.

2 of 3 reasons I use powertoys are power rename and image resizer. So your fix seems painful for me...

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 19, 2022
@zadjii-msft
Copy link
Member

zadjii-msft commented Jul 19, 2022

I'm gonna tentatively say that this is tracking upstream. If this comes back as not a PowerToys AND not an OS issue, then we can reopen if there's something for us to fix here. Thanks for following up!

microsoft/PowerToys#19473

@zadjii-msft zadjii-msft closed this as not planned Won't fix, can't repro, duplicate, stale Jul 19, 2022
@ghost

This comment was marked as outdated.

@ghost ghost closed this as completed Jul 19, 2022
@ghost ghost added Resolution-External For issues that are outside this codebase and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Attention The core contributors need to come back around and look at this ASAP. labels Jul 19, 2022
@zadjii-msft
Copy link
Member

I'm actually gonna reopen this. We've got like, a LOT of crashes in OpenTerminalHere::GetState

There's no internal bug for this yet (weird), but it is in 33573326-3f07-2517-b549-6f77533ee596.

HRESULT OpenTerminalHere::GetState(IShellItemArray* psiItemArray,
BOOL /*fOkToBeSlow*/,
EXPCMDSTATE* pCmdState)
{
// compute the visibility of the verb here, respect "fOkToBeSlow" if this is
// slow (does IO for example) when called with fOkToBeSlow == FALSE return
// E_PENDING and this object will be called back on a background thread with
// fOkToBeSlow == TRUE
// We however don't need to bother with any of that.
// If no item was selected when the context menu was opened and Explorer
// is not at a valid path (e.g. This PC or Quick Access), we should hide
// the verb from the context menu.
if (psiItemArray == nullptr)
{
const auto path = this->_GetPathFromExplorer();
*pCmdState = path.empty() ? ECS_HIDDEN : ECS_ENABLED;
}
else
{
winrt::com_ptr<IShellItem> psi;
psiItemArray->GetItemAt(0, psi.put());
SFGAOF attributes;
const bool isFileSystemItem = (psi->GetAttributes(SFGAO_FILESYSTEM, &attributes) == S_OK);
*pCmdState = isFileSystemItem ? ECS_ENABLED : ECS_HIDDEN;
}
return S_OK;
}

Maybe there's a way for the array to be non-null but have 0 items in it?

@zadjii-msft zadjii-msft reopened this Jul 28, 2022
@zadjii-msft zadjii-msft added Severity-Crash Crashes are real bad news. and removed Resolution-External For issues that are outside this codebase labels Jul 28, 2022
@zadjii-msft zadjii-msft added Product-Terminal The new Windows Terminal. Priority-2 A description (P2) Area-ShellExtension For issues related to the explorer right-click context menu labels Jul 28, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 28, 2022
@zadjii-msft zadjii-msft added this to the Terminal v1.16 milestone Jul 28, 2022
@zadjii-msft
Copy link
Member

Promoted to MSFT:40872815

probably regressed in #13206

@zadjii-msft zadjii-msft self-assigned this Aug 16, 2022
zadjii-msft added a commit that referenced this issue Aug 16, 2022
@zadjii-msft
Copy link
Member

I'm pretty sure that we fixed this in https://github.com/microsoft/terminal/releases/tag/v1.15.2282.0 by reverting #13206. Thanks all!

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Aug 18, 2022
DHowett added a commit that referenced this issue Sep 20, 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)

References #13206
References #13523
Closes #12578
DHowett added a commit that referenced this issue 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>
DHowett added a commit that referenced this issue 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
DHowett added a commit that referenced this issue 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-ShellExtension For issues related to the explorer right-click context menu Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

No branches or pull requests

3 participants