From 8a18fc1406370911f59a403b51894326c991af21 Mon Sep 17 00:00:00 2001 From: tlylt Date: Tue, 5 Apr 2022 08:44:36 +0800 Subject: [PATCH 1/4] Fix link behavior --- docs/devGuide/design/projectStructure.md | 2 +- packages/core/src/html/SiteLinkManager.js | 5 +++ packages/core/src/html/linkProcessor.js | 11 +++++- packages/core/src/utils/fsUtil.js | 5 ++- .../test/unit/html/SiteLinkManager.test.js | 20 ++++++++++ .../core/test/unit/html/linkProcessor.test.js | 37 +++++++++++++++++++ 6 files changed, 77 insertions(+), 3 deletions(-) diff --git a/docs/devGuide/design/projectStructure.md b/docs/devGuide/design/projectStructure.md index 5000eb9d62..dbe85e7c68 100644 --- a/docs/devGuide/design/projectStructure.md +++ b/docs/devGuide/design/projectStructure.md @@ -32,7 +32,7 @@ The MarkBind project is developed in a Test'; + const mockNode = cheerio.parseHTML(mockLink)[0]; + + const EXPECTED_RESULT = 'Should not validate mailto or tel links'; + + expect(siteLinkManager.collectIntraLinkToValidate(mockNode, mockCwf)).toEqual(EXPECTED_RESULT); +}); + +test('Test tel URL link', () => { + const siteLinkManager = getNewSiteLinkManager(); + const mockLink = 'Test'; + const mockNode = cheerio.parseHTML(mockLink)[0]; + + const EXPECTED_RESULT = 'Should not validate mailto or tel links'; + + expect(siteLinkManager.collectIntraLinkToValidate(mockNode, mockCwf)).toEqual(EXPECTED_RESULT); +}); + test('Test link for disabled intralink validation', () => { const siteLinkManager = getNewSiteLinkManager(); const mockLink = 'Test'; diff --git a/packages/core/test/unit/html/linkProcessor.test.js b/packages/core/test/unit/html/linkProcessor.test.js index 06430583f0..a851245a42 100644 --- a/packages/core/test/unit/html/linkProcessor.test.js +++ b/packages/core/test/unit/html/linkProcessor.test.js @@ -11,6 +11,7 @@ const json = { './css/main.css': '3', './devGuide/index.html': '4', './rawFile': '5', + './spaced folder/img.png': '6', }; fs.vol.fromJSON(json, './src'); @@ -180,6 +181,24 @@ test('Test valid file asset links (png)', () => { expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT); }); +test('Test valid file asset links (with %20 in the file path)', () => { + const mockLink = 'Test'; + const mockNode = cheerio.parseHTML(mockLink)[0]; + const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode); + const EXPECTED_RESULT = 'Intralink is a valid File Asset'; + + expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT); +}); + +test('Test valid file asset links (with space in the file path)', () => { + const mockLink = 'Test'; + const mockNode = cheerio.parseHTML(mockLink)[0]; + const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode); + const EXPECTED_RESULT = 'Intralink is a valid File Asset'; + + expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT); +}); + test('Test valid file asset links (css)', () => { // should be checked as file asset const mockLink = 'Test'; @@ -201,3 +220,21 @@ test('Test invalid link for non-existent file asset (css)', () => { expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT); }); + +test('Test non intralinks (mailto)', () => { + const mockLink = 'Test Email'; + const mockNode = cheerio.parseHTML(mockLink)[0]; + const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode); + const EXPECTED_RESULT = 'Not Intralink'; + + expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT); +}); + +test('Test non intralinks (tel)', () => { + const mockLink = 'Test Phone'; + const mockNode = cheerio.parseHTML(mockLink)[0]; + const mockResourcePath = linkProcessor.getDefaultTagsResourcePath(mockNode); + const EXPECTED_RESULT = 'Not Intralink'; + + expect(linkProcessor.validateIntraLink(mockResourcePath, mockCwf, mockConfig)).toEqual(EXPECTED_RESULT); +}); From ae68da2d62495db370a90c8154158499e98f936c Mon Sep 17 00:00:00 2001 From: tlylt Date: Tue, 5 Apr 2022 08:50:26 +0800 Subject: [PATCH 2/4] Update test cases --- packages/cli/test/functional/test_site/expected/testLinks.html | 2 +- .../functional/test_site/expected/testLinks.page-vue-render.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/functional/test_site/expected/testLinks.html b/packages/cli/test/functional/test_site/expected/testLinks.html index ed27b5122a..8f4c6463ad 100644 --- a/packages/cli/test/functional/test_site/expected/testLinks.html +++ b/packages/cli/test/functional/test_site/expected/testLinks.html @@ -214,7 +214,7 @@

AutolinksThese will be converted:

https://www.google.com

https://markbind.org

-

foobar@gmail.com

+

foobar@gmail.com

These will not be converted:

google.com

markbind.org

diff --git a/packages/cli/test/functional/test_site/expected/testLinks.page-vue-render.js b/packages/cli/test/functional/test_site/expected/testLinks.page-vue-render.js index 3f39ae3faf..073f28f12d 100644 --- a/packages/cli/test/functional/test_site/expected/testLinks.page-vue-render.js +++ b/packages/cli/test/functional/test_site/expected/testLinks.page-vue-render.js @@ -11,7 +11,7 @@ with(this){return _c('div',{staticClass:"bg-info display-4 text-center text-whit with(this){return _c('p',[_c('strong',[_v("Relative Link Test")]),_v(" This is a relative Intra-Site link in a layout (see "),_c('a',{attrs:{"href":"/test_site/index.html#heading-with-hidden-keyword"}},[_v("link")]),_v(")")])} },function anonymous( ) { -with(this){return _c('div',{staticClass:"fixed-header-padding",attrs:{"id":"content-wrapper"}},[_c('h1',{attrs:{"id":"autolinks"}},[_c('span',{staticClass:"anchor",attrs:{"id":"autolinks"}}),_v("Autolinks"),_c('a',{staticClass:"fa fa-anchor",attrs:{"href":"#autolinks","onclick":"event.stopPropagation()"}})]),_v(" "),_c('p',[_v("A URL with "),_c('code',{pre:true,attrs:{"class":"line-numbers hljs inline no-lang"}},[_v("http(s)://")]),_v(" head or an email address in plain text will be auto converted into clickable links.")]),_v(" "),_c('p',[_v("This functionality is inherited from markdown-it, with the setting of "),_c('code',{pre:true,attrs:{"class":"line-numbers hljs inline no-lang"}},[_v("fuzzyLink")]),_v(" turned off.")]),_v(" "),_c('p',[_c('strong',[_v("These will be converted:")])]),_v(" "),_c('p',[_c('a',{attrs:{"href":"https://www.google.com"}},[_v("https://www.google.com")])]),_v(" "),_c('p',[_c('a',{attrs:{"href":"https://markbind.org"}},[_v("https://markbind.org")])]),_v(" "),_c('p',[_c('a',{attrs:{"href":"/test_site/mailto:foobar@gmail.com"}},[_v("foobar@gmail.com")])]),_v(" "),_c('p',[_c('strong',[_v("These will not be converted:")])]),_v(" "),_c('p',[_v("google.com")]),_v(" "),_c('p',[_v("markbind.org")]),_v(" "),_c('p',[_v("foo@bar")]),_v(" "),_c('p',[_c('strong',[_v("Tricks to prevent autolink:")])]),_v(" "),_c('p',[_c('code',{pre:true,attrs:{"class":"line-numbers hljs inline no-lang"}},[_v("https://markbind.org")])]),_v(" "),_c('p',[_v("https://"),_c('span'),_v("markbind.org")]),_v(" "),_c('i',{staticClass:"fa fa-arrow-circle-up fa-lg d-print-none",attrs:{"id":"scroll-top-button","onclick":"handleScrollTop()","aria-hidden":"true"}})])} +with(this){return _c('div',{staticClass:"fixed-header-padding",attrs:{"id":"content-wrapper"}},[_c('h1',{attrs:{"id":"autolinks"}},[_c('span',{staticClass:"anchor",attrs:{"id":"autolinks"}}),_v("Autolinks"),_c('a',{staticClass:"fa fa-anchor",attrs:{"href":"#autolinks","onclick":"event.stopPropagation()"}})]),_v(" "),_c('p',[_v("A URL with "),_c('code',{pre:true,attrs:{"class":"line-numbers hljs inline no-lang"}},[_v("http(s)://")]),_v(" head or an email address in plain text will be auto converted into clickable links.")]),_v(" "),_c('p',[_v("This functionality is inherited from markdown-it, with the setting of "),_c('code',{pre:true,attrs:{"class":"line-numbers hljs inline no-lang"}},[_v("fuzzyLink")]),_v(" turned off.")]),_v(" "),_c('p',[_c('strong',[_v("These will be converted:")])]),_v(" "),_c('p',[_c('a',{attrs:{"href":"https://www.google.com"}},[_v("https://www.google.com")])]),_v(" "),_c('p',[_c('a',{attrs:{"href":"https://markbind.org"}},[_v("https://markbind.org")])]),_v(" "),_c('p',[_c('a',{attrs:{"href":"mailto:foobar@gmail.com"}},[_v("foobar@gmail.com")])]),_v(" "),_c('p',[_c('strong',[_v("These will not be converted:")])]),_v(" "),_c('p',[_v("google.com")]),_v(" "),_c('p',[_v("markbind.org")]),_v(" "),_c('p',[_v("foo@bar")]),_v(" "),_c('p',[_c('strong',[_v("Tricks to prevent autolink:")])]),_v(" "),_c('p',[_c('code',{pre:true,attrs:{"class":"line-numbers hljs inline no-lang"}},[_v("https://markbind.org")])]),_v(" "),_c('p',[_v("https://"),_c('span'),_v("markbind.org")]),_v(" "),_c('i',{staticClass:"fa fa-arrow-circle-up fa-lg d-print-none",attrs:{"id":"scroll-top-button","onclick":"handleScrollTop()","aria-hidden":"true"}})])} },function anonymous( ) { with(this){return _c('div',[_c('footer',[_c('h1',{attrs:{"id":"heading-in-footer-should-not-be-indexed"}},[_c('span',{staticClass:"anchor",attrs:{"id":"heading-in-footer-should-not-be-indexed"}}),_v("Heading in footer should not be indexed"),_c('a',{staticClass:"fa fa-anchor",attrs:{"href":"#heading-in-footer-should-not-be-indexed","onclick":"event.stopPropagation()"}})]),_v(" "),_c('div',{staticClass:"text-center"},[_v("\n This is a dynamic height footer that supports markdown "),_c('span',[_v("😄")]),_v("!\n ")])])])} From 0436622364f7629d4e743f7ec4533a813bf3eed6 Mon Sep 17 00:00:00 2001 From: tlylt Date: Tue, 5 Apr 2022 16:37:44 +0800 Subject: [PATCH 3/4] Refactor functions --- packages/core/src/html/SiteLinkManager.js | 9 +++---- packages/core/src/html/linkProcessor.js | 27 ++++++++++--------- .../test/unit/html/SiteLinkManager.test.js | 4 +-- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/packages/core/src/html/SiteLinkManager.js b/packages/core/src/html/SiteLinkManager.js index aaae59115e..dbd9b5aee9 100644 --- a/packages/core/src/html/SiteLinkManager.js +++ b/packages/core/src/html/SiteLinkManager.js @@ -50,11 +50,6 @@ class SiteLinkManager { return 'Should not validate'; } - const mailtoOrTelRegex = /^(?:mailto:|tel:)/i; - if (_.has(node.attribs, 'href') && mailtoOrTelRegex.test(node.attribs.href)) { - return 'Should not validate mailto or tel links'; - } - if (node.attribs) { const hasIntralinkValidationDisabled = _.has(node.attribs, 'no-validation'); if (hasIntralinkValidationDisabled) { @@ -63,6 +58,10 @@ class SiteLinkManager { } const resourcePath = linkProcessor.getDefaultTagsResourcePath(node); + if (!linkProcessor.isIntraLink(resourcePath)) { + return 'Should not validate'; + } + this._addToCollection(resourcePath, cwf); return 'Intralink collected to be validated later'; } diff --git a/packages/core/src/html/linkProcessor.js b/packages/core/src/html/linkProcessor.js index b8298bfdab..9101a2bb8e 100644 --- a/packages/core/src/html/linkProcessor.js +++ b/packages/core/src/html/linkProcessor.js @@ -37,18 +37,23 @@ function getResourcePathFromRoot(rootPath, fullResourcePath) { return fsUtil.ensurePosix(path.relative(rootPath, fullResourcePath)); } -function _convertRelativeLink(node, cwf, rootPath, baseUrl, resourcePath, linkAttribName) { - if (!resourcePath) { - return; - } +/** + * @param {string} resourcePath parsed from the node's relevant attribute + * @returns {boolean} whether the resourcePath is a valid intra-site link + */ +function isIntraLink(resourcePath) { + return resourcePath + && !urlUtil.isUrl(resourcePath) + && !resourcePath.startsWith('#') + && !/^(?:mailto:|tel:)/i.test(resourcePath); // mailto/tel links are not relative +} - const isMailtoOrTel = /^(?:mailto:|tel:)/i.test(resourcePath); - if (isMailtoOrTel) { - // mailto/tel links are not relative +function _convertRelativeLink(node, cwf, rootPath, baseUrl, resourcePath, linkAttribName) { + if (!isIntraLink(resourcePath)) { return; } - if (path.isAbsolute(resourcePath) || urlUtil.isUrl(resourcePath) || resourcePath.startsWith('#')) { + if (path.isAbsolute(resourcePath)) { // Do not rewrite. return; } @@ -154,10 +159,7 @@ function isValidFileAsset(resourcePath, config) { * @returns {string} these string return values are for unit testing purposes only */ function validateIntraLink(resourcePath, cwf, config) { - if (!resourcePath - || urlUtil.isUrl(resourcePath) - || resourcePath.startsWith('#') - || /^(?:mailto:|tel:)/i.test(resourcePath)) { + if (!isIntraLink(resourcePath)) { return 'Not Intralink'; } @@ -255,4 +257,5 @@ module.exports = { convertMdExtToHtmlExt, validateIntraLink, collectSource, + isIntraLink, }; diff --git a/packages/core/test/unit/html/SiteLinkManager.test.js b/packages/core/test/unit/html/SiteLinkManager.test.js index d735892639..8c0d2e232d 100644 --- a/packages/core/test/unit/html/SiteLinkManager.test.js +++ b/packages/core/test/unit/html/SiteLinkManager.test.js @@ -32,7 +32,7 @@ test('Test mailto URL link', () => { const mockLink = 'Test'; const mockNode = cheerio.parseHTML(mockLink)[0]; - const EXPECTED_RESULT = 'Should not validate mailto or tel links'; + const EXPECTED_RESULT = 'Should not validate'; expect(siteLinkManager.collectIntraLinkToValidate(mockNode, mockCwf)).toEqual(EXPECTED_RESULT); }); @@ -42,7 +42,7 @@ test('Test tel URL link', () => { const mockLink = 'Test'; const mockNode = cheerio.parseHTML(mockLink)[0]; - const EXPECTED_RESULT = 'Should not validate mailto or tel links'; + const EXPECTED_RESULT = 'Should not validate'; expect(siteLinkManager.collectIntraLinkToValidate(mockNode, mockCwf)).toEqual(EXPECTED_RESULT); }); From 1aa78af0abb8f72c401a037225c2ba8566a5006d Mon Sep 17 00:00:00 2001 From: tlylt Date: Tue, 5 Apr 2022 21:33:32 +0800 Subject: [PATCH 4/4] Update isIntraLink --- packages/core/src/html/linkProcessor.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/html/linkProcessor.js b/packages/core/src/html/linkProcessor.js index 9101a2bb8e..c2e675ec81 100644 --- a/packages/core/src/html/linkProcessor.js +++ b/packages/core/src/html/linkProcessor.js @@ -42,10 +42,11 @@ function getResourcePathFromRoot(rootPath, fullResourcePath) { * @returns {boolean} whether the resourcePath is a valid intra-site link */ function isIntraLink(resourcePath) { + const MAILTO_OR_TEL_REGEX = /^(?:mailto:|tel:)/i; return resourcePath && !urlUtil.isUrl(resourcePath) && !resourcePath.startsWith('#') - && !/^(?:mailto:|tel:)/i.test(resourcePath); // mailto/tel links are not relative + && !MAILTO_OR_TEL_REGEX.test(resourcePath); } function _convertRelativeLink(node, cwf, rootPath, baseUrl, resourcePath, linkAttribName) {