Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/js/_enqueues/lib/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,16 @@
inputs.url.on( 'blur', wpLink.correctURL );
},

// If URL wasn't corrected last time and doesn't start with http:, https:, ? # or /, prepend http://.
/*
* If the URL wasn't corrected last time and doesn't start with a scheme
* (a letter followed by letters, digits, `+` or `-`, then a colon, matching
* RFC 3986 minus the dot so that `example.com:8080` is still corrected),
* `#`, `?`, `.` or `/`, prepend http://.
*/
correctURL: function () {
var url = inputs.url.val().trim();

if ( url && correctedURL !== url && ! /^(?:[a-z]+:|#|\?|\.|\/)/.test( url ) ) {
if ( url && correctedURL !== url && ! /^(?:[a-zA-Z][a-zA-Z0-9+-]*:|#|\?|\.|\/)/.test( url ) ) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With this regex, a host and port containing a hyphen or a digit (e.g. my-site:8080/x or site1:80) no longer gets http:// prepended, although it did before. Unless there's a specific reason for this, I think we should keep the previous behavior. It would also be good to cover this in the unit tests.

One way to keep the previous behavior while still recognizing schemes like chrome-extension:// and web+demo2:// is to treat a scheme containing digits, + or - as a scheme only when it's followed by //:

Suggested change
if ( url && correctedURL !== url && ! /^(?:[a-zA-Z][a-zA-Z0-9+-]*:|#|\?|\.|\/)/.test( url ) ) {
if ( url && correctedURL !== url && ! /^(?:[a-z]+:|[a-z][a-z0-9+-]*:\/\/|#|\?|\.|\/)/i.test( url ) ) {
Input Result
my-site:8080/x, site1:80, example.com:8080/path http:// prepended (same as before)
chrome-extension://abc/o.html, web+demo2://example.com Unchanged
HTTPS://a.com, Http://a.org, mailto:a@b.c Unchanged
localhost:3000 Unchanged (same as before)

inputs.url.val( 'http://' + url );
correctedURL = url;
}
Expand Down
2 changes: 1 addition & 1 deletion src/js/_enqueues/vendor/tinymce/plugins/wplink/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@
return;
}

if ( ! /^(?:[a-z]+:|#|\?|\.|\/)/.test( href ) && ! emailRegex.test( href ) ) {
if ( ! /^(?:[a-zA-Z][a-zA-Z0-9+-]*:|#|\?|\.|\/)/.test( href ) && ! emailRegex.test( href ) ) {
href = 'http://' + href;
}

Expand Down
4 changes: 4 additions & 0 deletions tests/qunit/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
<body>
<div id="qunit"></div>
<div id="qunit-fixture">
<input id="wp-link-url" type="url" />
<script src="fixtures/customize-header.js"></script>
<script src="fixtures/customize-settings.js"></script>
<script src="fixtures/customize-menus.js"></script>
Expand All @@ -89,6 +90,8 @@
</div>

<!-- Tested files -->
<script>window.wpLinkL10n = window.wpLinkL10n || {};</script>
<script src="../../build/wp-includes/js/wplink.js"></script>
<script src="../../build/wp-admin/js/dashboard.js"></script>
<script src="../../build/wp-admin/js/password-strength-meter.js"></script>
<script src="../../build/wp-admin/js/postbox.js"></script>
Expand Down Expand Up @@ -205,6 +208,7 @@
<script src="wp-admin/js/widgets/test-media-video-widget.js"></script>
<script src="wp-includes/js/media/test-models.js"></script>
<script src="wp-includes/js/media/test-attachment-view.js"></script>
<script src="wp-includes/js/wplink.js"></script>

<!-- Customizer templates for sections -->
<script type="text/html" id="tmpl-customize-section-default">
Expand Down
98 changes: 98 additions & 0 deletions tests/qunit/wp-includes/js/wplink.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/* global wpLink, jQuery */

QUnit.module( 'wpLink.correctURL', {
beforeEach: function() {
// Re-initialize after QUnit resets the fixture, so inputs.url references the live element.
wpLink.init();
}
} );

QUnit.test( 'should not prepend http:// to a lowercase https:// URL', function( assert ) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks a bit verbose, since a whole block is added each time a new test case is needed. Instead, how about splitting the cases into two groups, URLs that should get http:// prepended and URLs that should not, and defining the values to test as arrays? For example:

[
	'https://example.com',
	'HTTPS://example.net',
	'Http://example.org',
	'chrome-extension://abcdef/options.html',
	'web+demo2://example.com',
	'#section',
	'/page/subpage/'
].forEach( function( url ) {
	QUnit.test( 'should not prepend http:// to ' + url, function( assert ) {
		jQuery( '#wp-link-url' ).val( url );
		wpLink.correctURL();
		assert.strictEqual( jQuery( '#wp-link-url' ).val(), url );
	} );
} );

[
	'example.com',
	'example.com:8080/path'
].forEach( function( url ) {
	QUnit.test( 'should prepend http:// to ' + url, function( assert ) {
		jQuery( '#wp-link-url' ).val( url );
		wpLink.correctURL();
		assert.strictEqual( jQuery( '#wp-link-url' ).val(), 'http://' + url );
	} );
} );

jQuery( '#wp-link-url' ).val( 'https://example.com' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'https://example.com',
'A lowercase https:// URL should not be modified.'
);
} );

QUnit.test( 'should not prepend http:// to an uppercase HTTPS:// URL', function( assert ) {
jQuery( '#wp-link-url' ).val( 'HTTPS://example.net' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'HTTPS://example.net',
'An uppercase HTTPS:// URL should not be modified (common on mobile keyboards).'
);
} );

QUnit.test( 'should not prepend http:// to a mixed-case Http:// URL', function( assert ) {
jQuery( '#wp-link-url' ).val( 'Http://example.org' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'Http://example.org',
'A mixed-case Http:// URL should not be modified.'
);
} );

QUnit.test( 'should prepend http:// to a bare domain without a scheme', function( assert ) {
jQuery( '#wp-link-url' ).val( 'example.com' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'http://example.com',
'A bare domain should get http:// prepended.'
);
} );

QUnit.test( 'should not prepend http:// to a scheme containing a hyphen', function( assert ) {
jQuery( '#wp-link-url' ).val( 'chrome-extension://abcdef/options.html' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'chrome-extension://abcdef/options.html',
'A scheme containing a hyphen should not be modified.'
);
} );

QUnit.test( 'should not prepend http:// to a scheme containing a plus sign and digits', function( assert ) {
jQuery( '#wp-link-url' ).val( 'web+demo2://example.com' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'web+demo2://example.com',
'A scheme containing a plus sign and digits should not be modified.'
);
} );

QUnit.test( 'should prepend http:// to a bare host and port', function( assert ) {
jQuery( '#wp-link-url' ).val( 'example.com:8080/path' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'http://example.com:8080/path',
'A bare host and port is not a scheme, so it should get http:// prepended.'
);
} );

QUnit.test( 'should not prepend http:// to a fragment URL', function( assert ) {
jQuery( '#wp-link-url' ).val( '#section' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'#section',
'A fragment URL should not be modified.'
);
} );

QUnit.test( 'should not prepend http:// to a root-relative URL', function( assert ) {
jQuery( '#wp-link-url' ).val( '/page/subpage/' );
wpLink.correctURL();
assert.strictEqual(
jQuery( '#wp-link-url' ).val(),
'/page/subpage/',
'A root-relative URL should not be modified.'
);
} );
Loading