Skip to content

[cmdpal] Fix TimeAndDate extension crash issue when typing query #40245

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
merged 1 commit into from
Jun 27, 2025

Conversation

moooyo
Copy link
Contributor

@moooyo moooyo commented Jun 26, 2025

Summary of the Pull Request

When we call RaiseItemsChanged, it will trigger function GetItems. So if you enter this function through typing query, it will trap in an inf loop. Eventually leading to crash.

related change: #40050

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@moooyo moooyo marked this pull request as ready for review June 26, 2025 06:48
@moooyo moooyo requested a review from htcfreek June 26, 2025 06:48
@moooyo moooyo added this to the PowerToys 0.92 milestone Jun 26, 2025
@moooyo moooyo added Product-Command Palette Refers to the Command Palette utility In for .92 labels Jun 26, 2025
@htcfreek
Copy link
Collaborator

do we really need all thos complicated stuff? Can't we simply get data an return it?

In my last pr i tried to fix outdated results by never having cache marked as up to date. but maybe it makes more sense to remove it at all.

@moooyo
Copy link
Contributor Author

moooyo commented Jun 26, 2025 via email

@moooyo
Copy link
Contributor Author

moooyo commented Jun 26, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@htcfreek
Copy link
Collaborator

If we are sure that every time the page is opened or the query changes we get live time results and not cached ones, I am okay with this code.

@moooyo
Copy link
Contributor Author

moooyo commented Jun 26, 2025

If we are sure that every time the page is opened or the query changes we get live time results and not cached ones, I am okay with this code.

Yeah, I can confirm it works. Will not break your previours PR.

Copy link
Contributor

@michaeljolley michaeljolley left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@moooyo moooyo merged commit 6d0af32 into microsoft:main Jun 27, 2025
9 checks passed
yeelam-gordon pushed a commit that referenced this pull request Jun 27, 2025
)

<!-- Enter a brief description/summary of your PR here. What does it
fix/what does it change/how was it tested (even manually, if necessary)?
-->
## Summary of the Pull Request
When we call RaiseItemsChanged, it will trigger function GetItems. So if
you enter this function through typing query, it will trap in an inf
loop. Eventually leading to crash.

related change: #40050

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist

- [x] **Closes:** #39973
- [x] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [x] **Tests:** Added/updated and all pass
- [ ] **Localization:** All end-user-facing strings can be localized
- [ ] **Dev docs:** Added/updated
- [ ] **New binaries:** Added on the required places
- [ ] [JSON for
signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json)
for new binaries
- [ ] [WXS for
installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs)
for new binaries and localization folder
- [ ] [YML for CI
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml)
for new test projects
- [ ] [YML for signed
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml)
- [ ] **Documentation updated:** If checked, please file a pull request
on [our docs
repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys)
and link it here: #xxx

<!-- Provide a more detailed description of the PR, other things fixed,
or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments

<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed

Co-authored-by: Yu Leng <yuleng@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In for .92 Product-Command Palette Refers to the Command Palette utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants