cnumr/EcoIndex

[Bug]: Automatically add protocol (http[s]://) to the url

rlemaire opened this issue · 11 comments

Que s'est il passé ?

I showed the new ecoindex to a friend yesterday and he typed something like "www.thesite.com". It didn't work and he then copy / pasted the url from his browser.

Sur quel navigateur avez vous rencontré ce problème ?

No response

Sur quel device rencontrez vous le problème ?

Smartphone

Un message d'erreur est il apparu ?

The URL was considered invalid.

Thanks for this feature report ;)
We were talking about it last week @aureliebaton and @yaaax ... OK, what should be the best option here?

  • Set a default input value to https://
  • Add a js feature to detect / fix an url that does not start with http:// / https://
  • Other idea ?

It can also be done in the backend with pattern matching.

jpkha commented

Actually there is already something for adding "https" on the code, develop by @yaaax .
https://github.com/cnumr/EcoIndex/blame/master/assets/js/services/AnalysisService.js
The issue is the validation pattern from the input.
By default the input with type="url" make this example "www.thesite.com" not valid.

jpkha commented

One of the solution is, changing type="url" to "text" and add a regex pattern like :

^(http(s)?:\/\/)?(?:\S+(?::\S*)?@)?(?:(?!(?:10|127)(?:\.\d{1,3}){3})(?!(?:169\.254|192\.168)(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,}))\.?)(?::\d{2,5})?(?:[/?#]\S*)?$

It'd probably be easier to handle validation in JS using a new URL(input.value) then.

Alternatively, keep the type=url, add a pattern="https?://.*", and handle the invalid event to possibly fix the value and re-submit the form (assuming that validation only happens when the form is submitted, but that's the case here).

Something like:

<input name=siteurl type=url pattern="https?://.*">
<script>
document.querySelector("input[type=url]").addEventListener("invalid", e => {
    if (e.target.validity.patternMismatch) {
        e.target.value = `https://${e.target.value}`;
        setTimeout(() => e.target.form.requestSubmit(), 0);
    }
});

(requestSubmit() will revalidate the field before submitting, so if you put, say a:b, it'll first be changed to https://a:b and then submission will fail again because https://a:b is not a valid URL)

Ideally, one would also fix basic mistakes like http:/www.thesite.com (add the missing slash). Using https?:/ (single slash) in the pattern would handle that case by not causing a patternMismatch but only a typeMismatch (so http:/www.thesite.com wouldn't be changed to https://http:/www.thesite.com, which incidentally is a valid URL!)

IMO I think that this should be handled on the frontend, because it has no added value to make a request to validate the url...

@aureliebaton what is the best UX implementation in this case ?

Other idea, @vvatelot!

In fact, there is no error here. Just an enhancement.

The first validation occurs with HTML (basic url scheme).
The second validation occurs with JS (prevent incorrect mime-type submission).

If the user hovers the field, he will read in a tooltip that he must add http(s) before the url.
All we have to do is show this message when he tries to submit with incomplete url, not only on hovering.

How? With setCustomValidity.

Should I try?

Other idea, @vvatelot!

In fact, there is no error here. Just an enhancement.

The first validation occurs with HTML (basic url scheme). The second validation occurs with JS (prevent incorrect mime-type submission).

If the user hovers the field, he will read in a tooltip that he must add http(s) before the url. All we have to do is show this message when he tries to submit with incomplete url, not only on hovering.

How? With setCustomValidity.

Should I try?

I like this approach. This seems to be a good compromize. Let's give it a try !

Oh! I was trying and I saw that in AnalysisService.js :

const domainNameRegex = /^(?:\S+(?::\S*)?@)?(?:(?!(?:10|127)(?:\.\d{1,3}){3})(?!(?:169\.254|192\.168)(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,}))\.?)(?::\d{2,5})?(?:[/?#]\S*)?$/;
if (!url.match(/^(http|https):\/\//) && domainNameRegex.test(url)) {
	url = `https://${url}`;
}

It seems that we just have to remove type=url in our main field to use this peace of code and automatically prepend https (not http) if needed.

jpkha commented

After a meeting, we decide to add a js to detect if a url have httpor https, if not, add automatically https://

Resolved by PR #266.