URL: Fix code drift in the Editor package by removing duplicate cleanForSlug function#39033
Merged
URL: Fix code drift in the Editor package by removing duplicate cleanForSlug function#39033
Conversation
…ForSlug function.
|
Size Change: -18 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
dmsnell
reviewed
Feb 23, 2022
…ences to use addQueryArgs
Contributor
Author
|
I also removed |
dmsnell
reviewed
Feb 23, 2022
Member
Contributor
Author
Interesting, @Mamaduka let me add some test cases. In this case, would there be any reason for URL permalink slugs (that use a post title) and named template part slugs to diverge in behavior? |
Contributor
Author
aristath
approved these changes
Feb 25, 2022
Contributor
Author
|
Excellent, thanks for the reviews folks! |
8 tasks
This file contains hidden or 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
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.
I was investigating reports that the object replacement character could be added to a post slug, and noticed that we had two definitions of
cleanForSlug, with diverging behavior.I fixed this by updating the function in the url package to include fixes from #24710, and re-exporting the function in the editor package. I think we can probably deprecate the url utils in the editor package (in either a follow up or in this PR), but let me know if folks have opinions on this.
Note that this is client-side sanitization, so this will help prevent new bad entries. For posts/slugs with existing bad characters we'll need a separate patch for server sanitization.
cc @claudiulodro @Robertght would it be possible to see if this also helps with: #38637
Testing Instructions
In console:
returns
'καλημέρα-κόσμε'instead of an empty string.Has the same result, but also includes a deprecation warning.