Fixes #2662 - URL with a space in the domain name should not be accepted

[Anushi1998]

Fixes #2662
Added a space check to validate URL.

r?@karlcow

[karlcow]

@Anushi1998 Two of the functional tests have failed. Could you double check? Thanks.

× firefox on linux 4.4.0-137-generic - Reporting (non-auth) - URL validation (10.96s)
    Timeout: An operation did not complete before its timeout expired.
      at Test.URL validation [as test]  <tests/functional/reporting-non-auth.js:93:10>
      at <src/lib/Test.ts:263:51>

× firefox on linux 4.4.0-137-generic - Reporting (non-auth) - Submits are enabled (0.666s)
    AssertionError: expected 'button button-secondary js-Button is-disabled' to not include 'is-disabled'
      at <tests/functional/reporting-non-auth.js:242:22>
      at Command.<anonymous>  <tests/functional/reporting-non-auth.js:241:24>
      at Test.Submits are enabled [as test]  <tests/functional/reporting-non-auth.js:240:12>
      at <src/lib/Test.ts:263:51>

[karlcow]

@Anushi1998 you need to double check this test.

webcompat.com/tests/functional/reporting-non-auth.js

Lines 68 to 95 in cc26a91

	"URL validation"() {
	return FunctionalHelpers.openPage(
	this,
	url("/issues/new"),
	".js-report-buttons"
	)
	.findByCssSelector("#url")
	.click()
	.type("sup")
	.end()
	.sleep(500)
	.findByCssSelector(".form-message-error")
	.getVisibleText()
	.then(function(text) {
	assert.include(
	text,
	"A valid URL is required",
	"URL validation message is shown"
	);
	})
	.end()
	.findByCssSelector("#url")
	.clearValue()
	.type("http://sup.com")
	.end()
	.waitForDeletedByCssSelector(".form-message-error")
	.end();
	},

[Anushi1998]

@karlcow
I am not able to reproduce the error on my system. I tried running npm run lint but there is no error.

As I don't have permission can you please SSH into CI Container and provide me two debug files. If there is another way around to download them please let me know that :)

[karlcow]

@Anushi1998 It's not an issue about linting. It's a functional test failing. And it fails specifically about the modification which was introduced with this PR.

In #2701 (comment) This is the link to the test webcompat.com/tests/functional/reporting-non-auth.js

It would be good to add an additional test with a URL with space. The functional tests are a series of programmatic steps using the UI and checking what should be the result.

If you want to run the functional tests locally, just follow the instructions in
https://github.com/webcompat/webcompat.com/blob/master/docs/tests.md#functional-tests

Hope it helps.

[Anushi1998]

@karlcow I have run npm run test and they are passing now. They were failing at that time because I have accidentally written https in both places instead of http.

The CircleCI says this now

> webcompat@ lint /home/circleci/repo
> npm run lint:js && npm run lint:css


> webcompat@ lint:js /home/circleci/repo
> npx eslint ./Gruntfile.js ./tests ./grunt-tasks ./webcompat/static/js/lib


/home/circleci/repo/webcompat/static/js/lib/editor.js
   7:2  error  Delete `···`  prettier/prettier
   8:2  error  Delete `···`  prettier/prettier
   9:2  error  Delete `···`  prettier/prettier
  10:2  error  Delete `···`  prettier/prettier
  11:2  error  Delete `···`  prettier/prettier
  12:1  error  Insert `·`    prettier/prettier
  29:2  error  Delete `···`  prettier/prettier
  30:2  error  Delete `···`  prettier/prettier
  31:2  error  Delete `···`  prettier/prettier
  32:1  error  Insert `·`    prettier/prettier

✖ 10 problems (10 errors, 0 warnings)
  10 errors and 0 warnings potentially fixable with the `--fix` option.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! webcompat@ lint:js: `npx eslint ./Gruntfile.js ./tests ./grunt-tasks ./webcompat/static/js/lib`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the webcompat@ lint:js script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/circleci/.npm/_logs/2018-11-07T02_47_00_096Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! webcompat@ lint: `npm run lint:js && npm run lint:css`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the webcompat@ lint script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/circleci/.npm/_logs/2018-11-07T02_47_00_104Z-debug.log
Exited with code 1

I will add a functional test which will check the URL with space, currently I have not included that :)

[Anushi1998]

[karlcow]

I have triggered another run on CircleCI to see if the tests are failing again.
npm run lint is checking only the syntax. :) not the functional tests.

[karlcow]

And this time it failed with more errors. :)
… https://circleci.com/gh/webcompat/webcompat.com/154

why is it complaining about /home/circleci/repo/webcompat/static/js/lib/editor.js

This PR is not touching to this file… or is it greenkeeper messing up.
Ah would it be #2700 by @miketaylr

[miketaylr]

Wondering if circleci cache is stale here...

[miketaylr]

OK, so https://stackoverflow.com/a/52075043/66348

This might be @Greenkeeper[bot] causing more problems.

c56a14c

On my machine:

🐓 npm ls | grep eslint
├─┬ [email protected]
│ ├─┬ [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ │ └── [email protected] deduped
├─┬ [email protected]
├─┬ [email protected]

🐓 npm ls | grep prettier
├─┬ [email protected]
├─┬ [email protected]
│ └─┬ [email protected]
├── [email protected]

Neat, I can ssh into the circleci container:

circleci@93c5d4864895:~/repo$ npm ls | grep eslint
├─┬ [email protected]
│ ├─┬ [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ │ └── [email protected] deduped
├─┬ [email protected]
├─┬ [email protected]

├─┬ [email protected]
├─┬ [email protected]
│ └─┬ [email protected]
├── [email protected]

Welp, not eslint. prettier is different on my machine and circle-ci.

@Anushi1998, can you past the output of those 2 commands here?

[miketaylr]

OK, so @Greenkeeper[bot] isn't to blame here. We just have a very vague range. I can send in a PR fixing it, you'll just need to rebase. Sorry about the confusion @Anushi1998.

[miketaylr]

Heh, if I had just checked the issues before doing the research, we'd know that @Greenkeeper[bot] figured it out for us. +1 to the bot: #2704

[miketaylr]

@Anushi1998, ok please rebase against the latest master and re-push -- tests should pass.

[Anushi1998]

@miketaylr Very sorry for unavailability. I just saw the mails.

Thanks for fixing it. I will rebase it against master and let you know the results :)

[miketaylr]

@miketaylr Very sorry for unavailability. I just saw the mails.

heh, no need to apologize. no rush at all.

[karlcow]

ok very cool @Anushi1998
This is working! Thanks for your patience.
Would it be possible to add a test with a space in the domain name.

Something like

 "space in domain name validation"() { 
   return FunctionalHelpers.openPage( 
     this, 
     url("/issues/new"), 
     ".js-report-buttons" 
   ) 
     .findByCssSelector("#url") 
     .click() 
     .type("http:// example.com") 
     .end() 
     .sleep(500) 
.findByCssSelector(".form-message-error") 

[Anushi1998]

Ok so I am done with adding tests now :)

r? @karlcow @miketaylr

Also @miketaylr would you mind telling how you can resolve the comment, I can't see an option for the same. Is it something to do with admin work/Github API?

[karlcow]

@Anushi1998 Thanks a lot! Cool and very useful work!

[Anushi1998]

:)

[miketaylr]

Also @miketaylr would you mind telling how you can resolve the comment, I can't see an option for the same. Is it something to do with admin work/Github API?

I actually don't know! GitHub might have some documentation, but it's not required to do so IMHO.