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.

Sign up for GitHub

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

Make linkValidation allow hash links #1143

Merged
merged 3 commits into from
Jul 20, 2016
Merged

Make linkValidation allow hash links #1143

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a357a67
Make linkValidation allow hash links
Jul 13, 2016
319233f
Consistent spacing between assertions
Jul 19, 2016
635becb
Allow void fragment identifier
Jul 20, 2016
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
31 changes: 31 additions & 0 deletions spec/anchor.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ describe('Anchor Button TestCase', function () {
});
expect(editor.elements[0].querySelector('a')).toBeNull();
});

it('should add http:// if need be and linkValidation option is set to true', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -220,6 +221,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe('http://test.com/');
});

it('should add tel: if need be and linkValidation option is set to true', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -237,6 +239,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe('tel:347-999-9999');
});

it('should not change protocol when a valid one is included', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -255,6 +258,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe(validUrl);
});

it('should not change protocol when a tel scheme is included', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -274,6 +278,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe(validUrl);
});

it('should not change protocol when a maps scheme is included', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -293,6 +298,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe(validUrl);
});

it('should not change protocol for protocol-relative URLs', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -311,6 +317,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe(window.location.protocol + validUrl);
});

it('should not change protocol for any alphabetic scheme', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -329,6 +336,26 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe(validUrl.toLowerCase());
});

it('should not change fragment identifier when link begins with hash', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
linkValidation: true
}
}),
validHashLink = '#!$&\'()*+,;=123abcDEF-._~:@/?',
link,
anchorExtension = editor.getExtensionByName('anchor');

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

link = editor.elements[0].querySelector('a');
expect(link).not.toBeNull();
expect(link.getAttribute('href')).toBe(validHashLink);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add an extra space before and after this assertion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I will add spaces.


it('should change spaces to %20 for a valid url if linkValidation options is set to true', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -354,6 +381,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.href).toBe(expectedOpts.value);
});

it('should not change spaces to %20 if linkValidation is set to false', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand Down Expand Up @@ -381,6 +409,7 @@ describe('Anchor Button TestCase', function () {
link = editor.elements[0].querySelector('a');
expect(link).not.toBeNull();
});

it('should add target="_blank" when "open in a new window" checkbox is checked', function () {
var editor = this.newMediumEditor('.editor', {
anchor: {
Expand All @@ -404,6 +433,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.target).toBe('_blank');
});

it('should add target="_blank" when respective option is set to true', function () {
var editor = this.newMediumEditor('.editor', {
targetBlank: true
Expand All @@ -419,6 +449,7 @@ describe('Anchor Button TestCase', function () {
expect(link).not.toBeNull();
expect(link.target).toBe('_blank');
});

it('should create a button when user selects this option and presses enter', function () {
spyOn(MediumEditor.prototype, 'createLink').and.callThrough();
var editor = this.newMediumEditor('.editor', {
Expand Down
3 changes: 2 additions & 1 deletion src/js/extensions/anchor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@
// Matches any alphabetical characters followed by ://
// Matches protocol relative "//"
// Matches common external protocols "mailto:" "tel:" "maps:"
var urlSchemeRegex = /^([a-z]+:)?\/\/|^(mailto|tel|maps):/i,
// Matches relative hash link, "#" followed by any character
var urlSchemeRegex = /^([a-z]+:)?\/\/|^(mailto|tel|maps):|^\#.+/i,
Copy link
Member

@nmielnik nmielnik Jul 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Should this allow just plain # as a valid link? Right now it seems like it wouldn't allow that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, the hash should be omitted if there is no fragment identifier; however, you are correct that it is valid, so it should be allowed here.

// var te is a regex for checking if the string is a telephone number
telRegex = /^\+?\s?\(?(?:\d\s?\-?\)?){3,20}$/;
if (telRegex.test(value)) {
Expand Down