-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
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
|
…ences to use addQueryArgs
I also removed |
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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @gwwar. Let's merge this, and I will handle template slug fixes in follow-up PRs.
Excellent, thanks for the reviews folks! |
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.