Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix: Packages\Url: Trim leading and trailing whitespaces #17320

Merged
merged 2 commits into from
Sep 4, 2019
Merged

Fix: Packages\Url: Trim leading and trailing whitespaces #17320

merged 2 commits into from
Sep 4, 2019

Conversation

donmhico
Copy link
Contributor

@donmhico donmhico commented Sep 4, 2019

Description

Trims the trailing and leading whitespaces in the url.

Fixes #17314.

How has this been tested?

  1. Create a new post.
  2. Insert a link with different url formats [ ] means whitespace.
  3. URLs above work as expected.

Types of changes

Bug fix.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.

@donmhico donmhico requested a review from talldan as a code owner September 4, 2019 00:47
@donmhico donmhico mentioned this pull request Sep 4, 2019
Closed
@talldan talldan added [Package] Format library /packages/format-library [Type] Bug An existing feature does not function as intended labels Sep 4, 2019
@gziolo gziolo added [Package] Url /packages/url and removed [Package] Format library /packages/format-library labels Sep 4, 2019
gziolo
gziolo reviewed Sep 4, 2019
View reviewed changes
packages/url/src/index.js
@@ -360,6 +360,7 @@ export function removeQueryArgs( url, ...args ) {
* @return {string} The updated URL.
*/
export function prependHTTP( url ) {
url = url.trim();
Copy link
Member

Choose a reason for hiding this comment

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

Can you add unit test which covers it?

gziolo
gziolo approved these changes Sep 4, 2019
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Cool, thanks for including unit test. I will merge as soon as Travis CI confirms everything is valid.

@gziolo gziolo added this to the Gutenberg 6.5 milestone Sep 4, 2019
@gziolo gziolo merged commit e420ac2 into WordPress:master Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo gziolo approved these changes

@talldan talldan Awaiting requested review from talldan

Assignees
No one assigned
Labels
[Package] Url /packages/url [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 6.5
Development

Successfully merging this pull request may close these issues.

Insert link: leading spaces pasted into hyperlink modal not stripped out
3 participants
@donmhico @gziolo @talldan