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.

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

Link UI too open as to what it considers a valid URL #38497

Closed
getdave opened this issue Feb 3, 2022 · 8 comments
Closed

Link UI too open as to what it considers a valid URL #38497

getdave opened this issue Feb 3, 2022 · 8 comments
Assignees
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Type] Bug An existing feature does not function as intended

Comments

@getdave
Copy link
Contributor

getdave commented Feb 3, 2022

What problem does this address?

When the Link UI <LinkControl> was created we decided to take a very broad definition as to what is considered a valid URL. Therefore you can type in a simple string with no spaces like Projects and (assuming you have no Posts/Pages with that name) the link UI will "detect" it as a URL and show an option below.

Screen Capture on 2022-02-03 at 16-28-17

Unfortunately this is potentially very confusing UX. Indeed I've seen users assuming this means they either:

  1. have a page called "Projects"
  2. they have magically created one

...even though nothing of the sort is happening. Instead what happens is an anchor tag is created with the href="Projects" which is clearly invalid.

Why does this happen? Well the rules around what is considered a URL are very broad. This allows strings such as the above to slip through the net.

What is your proposed solution?

I recommend we make the rules around what is considered a valid entry into the Link UI stricter. For example we could require inclusion of any of the follow to be considered a URL:

  • a top level domain (match against format .somethinghere using regex). For example .com or .org.
  • use of the www. prefix
  • use of a http(s) prefix.

Obviously we'd also need to include other protocols such as mailto and tel and internal links - many of which are already handled.

If none of those are matched then we could flag the entry as invalid.

I do think however it would be risky to disable the ability of the user to add it. This is because there is a very wide range of URI formats available and it would be quite hard to accomodate them all. Therefore we should simply flag any entry that appears to be outside of the typical URI a user might enter.

We could display a inline error message and ask the user are they sure they want to insert....etc

@getdave getdave added [Type] Bug An existing feature does not function as intended [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) labels Feb 3, 2022
@getdave getdave self-assigned this Feb 3, 2022
@draganescu
Copy link
Contributor

2 cents: the trend I noticed is that links get auto created if the string contains a dot.

@mrfoxtalbot
Copy link

I do not think it is practical to force validation on what users can enter as a URL, there are probably too many edge cases.

The ideall path would be to offer the possibility to create a page directly, as mentioned in #38493

@noisysocks
Copy link
Member

I think it's fine to update the component so that it treats these as URLS:

  • A string that matches the URL RFC, e.g. http://example.com, mailto:[email protected]
  • A string that contains no spaces and a period (.), e.g. example.com

We should try to optimise for the most common use cases. It is very uncommon that a user needs to link to http://Example. If a user really needs to do that, they can enter the http: prefix.

@talldan
Copy link
Contributor

talldan commented Feb 4, 2022

The good news is there already lots of URL validation utilities in the codebase (in the @wordpress/url package) for this. Originally implemented in #11286. These should closely follow the spec.

Validation is pretty complicated, and there a lot of edge cases! It's not just urls that have to be supported, but also hrefs like #my-heading.

@getdave
Copy link
Contributor Author

getdave commented Feb 4, 2022

Yeh the validation thing is why I think we ended up going for a pretty broad spec. Perhaps what @mrfoxtalbot suggests will help mitigate this issue.

@talldan
Copy link
Contributor

talldan commented Feb 7, 2022

Was just testing how it works now, looks like the changes previously introduced in #11286 are no longer present. Users can enter any value for an href and not know anything is wrong. I think there should at least be a warning shown for invalid hrefs, especially considering the code is already written and it just need to be reimplemented.

Having said that, what's described in the issue here doesn't seem like validation, more about determining user intent.

It might be an idea to split this into two two tasks, the first about implementing some loose rules to determine user intent (are they adding a URL? Are they adding an internal link? Is it the name of a page?).

The second task would be about showing a warning if the URL or internal link doesn't pass more strict checks (reimplementing #11286 for LinkControl, and hopefully showing a visible error this time as well as the red colored text).

@getdave
Copy link
Contributor Author

getdave commented May 10, 2023

Was just testing how it works now, looks like the changes previously introduced in #11286 are no longer present.

That's really interesting. Thank you.

It might be an idea to split this into two two tasks,

Good suggestion.

the first about implementing some loose rules to determine user intent (are they adding a URL? Are they adding an internal link? Is it the name of a page?).

Yes that's the key thing. So it's less "is this a URL" more "does the user possibly intend that this is a URL" (and similar for Pages...etc).

The first step is defining the heuristic and criteria to determine which is which. I'll start looking at that.

The second task would be about showing a warning if the URL or internal link doesn't pass more strict checks (reimplementing #11286 for LinkControl, and hopefully showing a visible error this time as well as the red colored text).

Yes and here is where we can provide validation of the any custom links.

@getdave
Copy link
Contributor Author

getdave commented Jun 6, 2023

Fixed by #51011

@getdave getdave closed this as completed Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

5 participants