Skip to content

Commit

Permalink
Don't encode fragment as part of linkValidation
Browse files Browse the repository at this point in the history
Fix for: #1237

Previously URIs were split at a question mark to determine path and
query components. This caused a problem when fragments were part of a
link as they were deemed to be part of the query.

This splits a URL at a ? and a # to get both components and does not
encode the fragment. It did feel a little wrong not to encode the
fragment at all as there some characters that should be encoded, however
the encodeUriComponent method is too heavy handed for a fragment.
  • Loading branch information
kevindew committed Dec 9, 2016
1 parent 34f070f commit 1168710
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
26 changes: 26 additions & 0 deletions spec/anchor.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,32 @@ describe('Anchor Button TestCase', function () {
expect(link.href).toBe(expectedOpts.value);
});

it('should not change # to %23 for a valid url if linkValidation option is set to true', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
linkValidation: true
}
}),
link,
anchorExtension = editor.getExtensionByName('anchor'),
expectedOpts = {
value: 'http://example.com/?query=value#anchor',
target: '_self'
};

spyOn(editor, 'execAction').and.callThrough();

selectElementContentsAndFire(editor.elements[0]);
anchorExtension.showForm(expectedOpts.value);
fireEvent(anchorExtension.getForm().querySelector('a.medium-editor-toolbar-save'), 'click');

expect(editor.execAction).toHaveBeenCalledWith('createLink', expectedOpts);

link = editor.elements[0].querySelector('a');
expect(link).not.toBeNull();
expect(link.href).toBe(expectedOpts.value);
});

it('should not encode an encoded URL if linkValidation option is set to true', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand Down
12 changes: 8 additions & 4 deletions src/js/extensions/anchor.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,10 @@
var urlSchemeRegex = /^([a-z]+:)?\/\/|^(mailto|tel|maps):|^\#/i,
// telRegex is a regex for checking if the string is a telephone number
telRegex = /^\+?\s?\(?(?:\d\s?\-?\)?){3,20}$/,
split = value.split('?'),
path = split[0],
query = split[1];
urlParts = value.match(/^(.*?)(?:\?(.*?))?(?:#(.*))?$/),
path = urlParts[1],
query = urlParts[2],
fragment = urlParts[3];

if (telRegex.test(value)) {
return 'tel:' + value;
Expand All @@ -271,7 +272,10 @@
// Ensure path is encoded
this.ensureEncodedUri(path) +
// Ensure query is encoded
(query === undefined ? '' : '?' + this.ensureEncodedQuery(query));
(query === undefined ? '' : '?' + this.ensureEncodedQuery(query)) +
// Include fragment unencoded as encodeUriComponent is too
// heavy handed for the many characters allowed in a fragment
(fragment === undefined ? '' : '#' + fragment);
}
},

Expand Down

0 comments on commit 1168710

Please sign in to comment.