From 05d5649c9c83e8a7c47d844e1e906e429b08768b Mon Sep 17 00:00:00 2001 From: skycommand Date: Wed, 9 Mar 2022 17:36:00 +0330 Subject: [PATCH] [MonacoPreviewHandler][Settings] Resolve preview handler race (#16180) * Register .markdown with the correct handler * Fix spelling * Move file name extensions from "expect.txt" to "excludes.txt" * Revert "Move file name extensions from "expect.txt" to "excludes.txt"" This reverts commit 710d5a4968c1c999a8157ad38167a88415dd8c5d. I must have misunderstood the instructions. * Revert "Register .markdown with the correct handler" This reverts commit 5c37b009f375a6369b168cec2e8b375a75fb24f5. * Work in progress * Code ready for testing * Update excludes.txt * Update excludes.txt * Update modulesRegistry.h * Update modulesRegistry.h For the want of an exclamation mark, a kingdom is lost! * Update modulesRegistry.h * Work on modulesRegistry.h per code review in 16180 Removed all previous exclusions from Monaco preview handler. Added a new exclusion: SVGZ. It's a binary file that Monaco cannot, in any meaningful way, read. * Update expect.txt * Update accessory files * Disable machine-wide checks for performance reasons --- .github/actions/spell-check/excludes.txt | 1 + .github/actions/spell-check/expect.txt | 6 ++ src/common/utils/modulesRegistry.h | 53 +++++++++---- .../Settings.UI/Strings/en-us/Resources.resw | 2 +- .../Check preview handler registration.ps1 | 76 +++++++++++++++++++ 5 files changed, 121 insertions(+), 17 deletions(-) create mode 100644 tools/Verification scripts/Check preview handler registration.ps1 diff --git a/.github/actions/spell-check/excludes.txt b/.github/actions/spell-check/excludes.txt index 1db7be9475..111e57704b 100644 --- a/.github/actions/spell-check/excludes.txt +++ b/.github/actions/spell-check/excludes.txt @@ -46,6 +46,7 @@ ignore$ ^src/modules/previewpane/PreviewPaneUnitTests/HelperFiles/MarkdownWithHTMLImageTag\.txt$ ^src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HelperFiles/MarkdownWithHTMLImageTag.txt$ ^tools/CleanUp_tool/CleanUp_tool\.vcxproj\.filters$ +^tools/Verification scripts/Check preview handler registration\.ps1$ ^\.github/ ^\.github/actions/spell-check/ ^\.gitmodules$ diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 23a7bf2c5b..e173062fbe 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -1179,6 +1179,9 @@ MAXIMIZEBOX MAXSHORTCUTSIZE maxversiontested MBs +mdtext +mdtxt +mdwn MBUTTON MBUTTONDBLCLK MBUTTONDOWN @@ -1225,6 +1228,8 @@ Miracast mirophone Mishkeegogamang mjpg +mkd +mkdn mlcfg MMDDYYYY mmdeviceapi @@ -1961,6 +1966,7 @@ SVE SVGIn SVGIO svgpreviewhandler +svgz SWC SWFO SWP diff --git a/src/common/utils/modulesRegistry.h b/src/common/utils/modulesRegistry.h index c9c517da5d..07ff5d94c9 100644 --- a/src/common/utils/modulesRegistry.h +++ b/src/common/utils/modulesRegistry.h @@ -13,8 +13,14 @@ namespace NonLocalizable const static wchar_t* MONACO_LANGUAGES_FILE_NAME = L"modules\\FileExplorerPreview\\monaco_languages.json"; const static wchar_t* ListID = L"list"; const static wchar_t* ExtensionsID = L"extensions"; - const static wchar_t* MDExtension = L".md"; - const static wchar_t* SVGExtension = L".svg"; + const static std::vector ExtSVG = { L".svg" }; + const static std::vector ExtMarkdown = { L".md", L".markdown", L".mdown", L".mkdn", L".mkd", L".mdwn", L".mdtxt", L".mdtext" }; + const static std::vector ExtPDF = { L".pdf" }; + const static std::vector ExtGCode = { L".gcode" }; + const static std::vector ExtSTL = { L".stl" }; + const static std::vector ExtNoNoNo = { + L".svgz" //Monaco cannot handle this file type at all; it's a binary file. + }; } inline registry::ChangeSet getSvgPreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser) @@ -30,7 +36,7 @@ inline registry::ChangeSet getSvgPreviewHandlerChangeSet(const std::wstring inst registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.PreviewHandler.Svg.SvgPreviewHandler", L"Svg Preview Handler", - { L".svg" }); + NonLocalizable::ExtSVG); } inline registry::ChangeSet getMdPreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser) @@ -44,14 +50,23 @@ inline registry::ChangeSet getMdPreviewHandlerChangeSet(const std::wstring insta registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.PreviewHandler.Markdown.MarkdownPreviewHandler", L"Markdown Preview Handler", - { L".md" }); + NonLocalizable::ExtMarkdown); } inline registry::ChangeSet getMonacoPreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser) { using namespace registry::shellex; + + // Set up a list of extensions for the preview handler to take over std::vector extensions; + // Set up a list of extensions that Monaco support but the preview handler shouldn't take over + std::vector ExtExclusions; + ExtExclusions.insert(ExtExclusions.end(), NonLocalizable::ExtMarkdown.begin(), NonLocalizable::ExtMarkdown.end()); + ExtExclusions.insert(ExtExclusions.end(), NonLocalizable::ExtSVG.begin(), NonLocalizable::ExtSVG.end()); + ExtExclusions.insert(ExtExclusions.end(), NonLocalizable::ExtNoNoNo.begin(), NonLocalizable::ExtNoNoNo.end()); + bool IsExcluded = false; + std::wstring languagesFilePath = fs::path{ installationDir } / NonLocalizable::MONACO_LANGUAGES_FILE_NAME; auto json = json::from_file(languagesFilePath); @@ -68,13 +83,19 @@ inline registry::ChangeSet getMonacoPreviewHandlerChangeSet(const std::wstring i for (uint32_t j = 0; j < extensionsList.Size(); ++j) { auto extension = extensionsList.GetStringAt(j); - - // Ignore extensions we already have dedicated handlers for - if (std::wstring{ extension } == std::wstring{ NonLocalizable::MDExtension } || - std::wstring{ extension } == std::wstring{ NonLocalizable::SVGExtension }) + + // Ignore extensions in the exclusion list + IsExcluded = false; + + for (std::wstring k : ExtExclusions) { - continue; + if (std::wstring{ extension } == k) + { + IsExcluded = true; + break; + } } + if (IsExcluded) { continue; } extensions.push_back(std::wstring{ extension }); } } @@ -106,7 +127,7 @@ inline registry::ChangeSet getPdfPreviewHandlerChangeSet(const std::wstring inst registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.PreviewHandler.Pdf.PdfPreviewHandler", L"Pdf Preview Handler", - { L".pdf" }); + NonLocalizable::ExtPDF); } inline registry::ChangeSet getGcodePreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser) @@ -120,7 +141,7 @@ inline registry::ChangeSet getGcodePreviewHandlerChangeSet(const std::wstring in registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.PreviewHandler.Gcode.GcodePreviewHandler", L"G-code Preview Handler", - { L".gcode" }); + NonLocalizable::ExtGCode); } inline registry::ChangeSet getSvgThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser) @@ -134,7 +155,7 @@ inline registry::ChangeSet getSvgThumbnailHandlerChangeSet(const std::wstring in registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.ThumbnailHandler.Svg.SvgThumbnailProvider", L"Svg Thumbnail Provider", - { L".svg" }); + NonLocalizable::ExtSVG); } inline registry::ChangeSet getPdfThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser) @@ -148,7 +169,7 @@ inline registry::ChangeSet getPdfThumbnailHandlerChangeSet(const std::wstring in registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.ThumbnailHandler.Pdf.PdfThumbnailProvider", L"Pdf Thumbnail Provider", - { L".pdf" }); + NonLocalizable::ExtPDF); } inline registry::ChangeSet getGcodeThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser) @@ -162,7 +183,7 @@ inline registry::ChangeSet getGcodeThumbnailHandlerChangeSet(const std::wstring registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.ThumbnailHandler.Gcode.GcodeThumbnailProvider", L"G-code Thumbnail Provider", - { L".gcode" }); + NonLocalizable::ExtGCode); } inline registry::ChangeSet getStlThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser) @@ -176,7 +197,7 @@ inline registry::ChangeSet getStlThumbnailHandlerChangeSet(const std::wstring in registry::DOTNET_COMPONENT_CATEGORY_CLSID, L"Microsoft.PowerToys.ThumbnailHandler.Stl.StlThumbnailProvider", L"Stl Thumbnail Provider", - { L".stl" }); + NonLocalizable::ExtSTL); } inline std::vector getAllModulesChangeSets(const std::wstring installationDir) @@ -191,4 +212,4 @@ inline std::vector getAllModulesChangeSets(const std::wstri getPdfThumbnailHandlerChangeSet(installationDir, PER_USER), getGcodeThumbnailHandlerChangeSet(installationDir, PER_USER), getStlThumbnailHandlerChangeSet(installationDir, PER_USER) }; -} \ No newline at end of file +} diff --git a/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw b/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw index 73a5dfe714..c5a4b2c939 100644 --- a/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw +++ b/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw @@ -617,7 +617,7 @@ Show recently used strings - Enable Markdown (.md) preview + Enable Markdown preview (.md, .markdown, .mdown, .mkdn, .mkd, .mdwn, .mdtxt, .mdtext) Do not loc "Markdown". Do you want this feature on / off diff --git a/tools/Verification scripts/Check preview handler registration.ps1 b/tools/Verification scripts/Check preview handler registration.ps1 new file mode 100644 index 0000000000..8790acd1ce --- /dev/null +++ b/tools/Verification scripts/Check preview handler registration.ps1 @@ -0,0 +1,76 @@ +#Requires -Version 7.2 + +using namespace System.Management.Automation + +[CmdletBinding()] +param() + +function PublicStaticVoidMain { + [CmdletBinding()] + param () + + class TypeHandlerData { + [String] $Name + [String] $CurrentUserHandler + [String] $MachineWideHandler + } + + [String[]]$TypesToCheck = @(".markdown", ".mdtext", ".mdtxt", ".mdown", ".mkdn", ".mdwn", ".mkd", ".md", ".svg", ".svgz", ".pdf", ".gcode", ".stl", ".txt", ".ini") + $IPREVIEW_HANDLER_CLSID = '{8895b1c6-b41f-4c1c-a562-0d564250836f}' + $PowerToysHandlers = @{ + '{07665729-6243-4746-95b7-79579308d1b2}' = "PowerToys PDF handler" + '{ddee2b8a-6807-48a6-bb20-2338174ff779}' = "PowerToys SVG handler" + '{ec52dea8-7c9f-4130-a77b-1737d0418507}' = "PowerToys GCode handler" + '{45769bcc-e8fd-42d0-947e-02beef77a1f5}' = "PowerToys Markdown handler" + '{afbd5a44-2520-4ae0-9224-6cfce8fe4400}' = "PowerToys Monaco fallback handler" + '{DC6EFB56-9CFA-464D-8880-44885D7DC193}' = "Adobe Acrobat DC" + } + + function ResolveHandlerGUIDtoName { + param ( + [Parameter(Mandatory, Position = 0)] + [String] $GUID + ) + return $PowerToysHandlers[$GUID] ?? $GUID + } + + function WriteMyProgress { + param ( + [Parameter(Mandatory, Position=0)] [Int32] $ItemsPending, + [Parameter(Mandatory, Position=1)] [Int32] $ItemsTotal, + [switch] $Completed + ) + [Int32] $PercentComplete = ($ItemsPending / $ItemsTotal) * 100 + if ($PercentComplete -lt 1) { $PercentComplete = 1} + Write-Progress -Activity 'Querying Windows Registry' -Status "$ItemsPending of $ItemsTotal" -PercentComplete $PercentComplete -Completed:$Completed + } + + $ItemsTotal = $TypesToCheck.Count * 2 + $ItemsPending = 0 + WriteMyProgress 0 $ItemsTotal + + $CheckResults = New-Object -TypeName 'System.Collections.Generic.List[TypeHandlerData]' + foreach ($item in $TypesToCheck) { + $CurrentUserGUID = Get-ItemPropertyValue -Path "HKCU://Software/Classes/$item/shellex/$IPREVIEW_HANDLER_CLSID" -Name '(default)' -ErrorAction SilentlyContinue + $ItemsPending += 1 + WriteMyProgress $ItemsPending $ItemsTotal + + $MachineWideGUID = "Didn't check" + # $MachineWideGUID = Get-ItemPropertyValue -Path "HKLM://Software/Classes/$item/shellex/$IPREVIEW_HANDLER_CLSID" -Name '(default)' -ErrorAction SilentlyContinue + $ItemsPending += 1 + WriteMyProgress $ItemsPending $ItemsTotal + + $temp = New-Object -TypeName TypeHandlerData -Property @{ + Name = $item + CurrentUserHandler = ($null -eq $CurrentUserGUID) ? "Nothing" : (ResolveHandlerGUIDtoName ($CurrentUserGUID)) + MachineWideHandler = ($null -eq $MachineWideGUID) ? "Nothing" : (ResolveHandlerGUIDtoName ($MachineWideGUID)) + } + $CheckResults.Add($temp) + + Clear-Variable 'CurrentUserGUID', 'MachineWideGUID' + } + WriteMyProgress $ItemsPending $ItemsTotal -Completed + $CheckResults | Format-Table -AutoSize +} + +PublicStaticVoidMain