Skip to content

[release/7.0] [wasm][debugger] Don't escape accented letters - #78217

Merged
carlossanlop merged 5 commits into
dotnet:release/7.0from
thaystg:thays_backport_78171
Nov 15, 2022
Merged

[release/7.0] [wasm][debugger] Don't escape accented letters#78217
carlossanlop merged 5 commits into
dotnet:release/7.0from
thaystg:thays_backport_78171

Conversation

@thaystg

@thaystgthaystg commented Nov 11, 2022

Copy link
Copy Markdown
Member

Backport of #78171 to release/7.0

/cc @thaystg

Customer Impact

When the customer adds a breakpoint in a file that contains any special character like (swedish characters) and the breakpoint is hit, the file is not opened correctly on VS.

Testing

Manually tested using a Blazor app, adding special characters in the folder, in the project name and in the source file name.

Risk

Low risk, do not escape all the letters and not only the ones unaccented.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@ghostghost assigned thaystgNov 11, 2022
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #78171 to release/7.0

/cc @thaystg

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:thaystg
Assignees:thaystg
Labels:

area-Debugger-mono

Milestone:-

@thaystg
thaystg requested a review from lewingNovember 11, 2022 12:34
@thaystgthaystg added the Servicing-consider Issue for next servicing release review label Nov 11, 2022
@thaystg

Copy link
Copy Markdown
MemberAuthor

Please, do not merge.

@radicalradical added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 14, 2022
@thaystgthaystg changed the title [release/7.0] [wasm][debugger] Don't need to escape special characters anymore[release/7.0] [wasm][debugger] Don't need to escape any letter or digitNov 14, 2022
@radicalradical removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 14, 2022

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes look fine for 7.0 branch. @thaystg will explore using Uri.EscapeDataString, and friends to handle the escaping in follow up PRs.

@thaystgthaystg changed the title [release/7.0] [wasm][debugger] Don't need to escape any letter or digit[release/7.0] [wasm][debugger] Don't escape accented lettersNov 14, 2022
@carlossanlop

Copy link
Copy Markdown
Contributor

This was approved by Tactics via email and is signed off by area owner. No OOB package authoring changes needed for this.
I'm just waiting for the CI to finish before merging.

@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 15, 2022
@carlossanlop

Copy link
Copy Markdown
Contributor

Failures happened all in the installer legs, none seem related to this change (it's all network/infra issues).

@carlossanlop
carlossanlop merged commit c38b76a into dotnet:release/7.0Nov 15, 2022
@ghostghost locked as resolved and limited conversation to collaborators Dec 15, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Debugger-monoServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@thaystg@carlossanlop@radical