From 045d170e828b5c3708283ada66db9b8d4ec1b3ac Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Wed, 28 Feb 2024 11:37:11 +0200 Subject: [PATCH 1/9] fix for fragments leading to invalid html --- libs/blocks/fragment/fragment.js | 6 ++++++ test/blocks/fragment/fragment.test.js | 9 +++++++++ test/blocks/fragment/mocks/body.html | 3 +++ test/blocks/fragment/mocks/fragments/frag-p.plain.html | 7 +++++++ 4 files changed, 25 insertions(+) create mode 100644 test/blocks/fragment/mocks/fragments/frag-p.plain.html diff --git a/libs/blocks/fragment/fragment.js b/libs/blocks/fragment/fragment.js index 5fac322d17..f87fb86c8c 100644 --- a/libs/blocks/fragment/fragment.js +++ b/libs/blocks/fragment/fragment.js @@ -63,6 +63,12 @@ export default async function init(a) { let relHref = localizeLink(a.href); let inline = false; + if (a.parentElement && a.parentElement.nodeName === 'P') { + const div = document.createElement('div'); + a.parentElement.replaceWith(div); + div.appendChild(a); + } + if (a.href.includes('#_inline')) { inline = true; a.href = a.href.replace('#_inline', ''); diff --git a/test/blocks/fragment/fragment.test.js b/test/blocks/fragment/fragment.test.js index 2e734c40c7..842be015ad 100644 --- a/test/blocks/fragment/fragment.test.js +++ b/test/blocks/fragment/fragment.test.js @@ -77,4 +77,13 @@ describe('Fragments', () => { const pic = document.querySelector('picture.frag-image'); expect(pic.classList.contains('decorated')).to.be.true; }); + + it('only valid HTML should exist after resolving the fragments', async () => { + const { body } = new DOMParser().parseFromString(await readFile({ path: './mocks/body.html' }), 'text/html'); + for (const a of body.querySelectorAll('a[href*="/fragment"]')) await getFragment(a); + const innerHtml = body.innerHTML; + // eslint-disable-next-line + body.innerHTML = body.innerHTML; // after reassignment, the parser guarantees the presence of only valid HTML + expect(innerHtml).to.equal(body.innerHTML); + }); }); diff --git a/test/blocks/fragment/mocks/body.html b/test/blocks/fragment/mocks/body.html index 9eb799258a..9a89357518 100644 --- a/test/blocks/fragment/mocks/body.html +++ b/test/blocks/fragment/mocks/body.html @@ -5,6 +5,9 @@ Fragment Fragment Fragment +

+ Fragment +

diff --git a/test/blocks/fragment/mocks/fragments/frag-p.plain.html b/test/blocks/fragment/mocks/fragments/frag-p.plain.html new file mode 100644 index 0000000000..2e360dbd86 --- /dev/null +++ b/test/blocks/fragment/mocks/fragments/frag-p.plain.html @@ -0,0 +1,7 @@ +
+
    +
  • List item
  • +
  • List item
  • +
  • List item
  • +
+
\ No newline at end of file From fa1429d82d4ba044f3e52f80b5049acd9904e9a3 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Wed, 28 Feb 2024 16:33:41 +0200 Subject: [PATCH 2/9] removed fragments fix from gnav --- libs/blocks/global-navigation/utilities/utilities.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/libs/blocks/global-navigation/utilities/utilities.js b/libs/blocks/global-navigation/utilities/utilities.js index 590552ef24..46aa01f4ea 100644 --- a/libs/blocks/global-navigation/utilities/utilities.js +++ b/libs/blocks/global-navigation/utilities/utilities.js @@ -294,18 +294,11 @@ export async function fetchAndProcessPlainHtml({ url, shouldDecorateLinks = true const inlineFrags = [...body.querySelectorAll('a[href*="#_inline"]')]; if (inlineFrags.length) { const { default: loadInlineFrags } = await import('../../fragment/fragment.js'); - const fragPromises = inlineFrags.map((link) => { - // Replacing paragraphs should happen in the fragment module - // https://jira.corp.adobe.com/browse/MWPW-141039 - if (link.parentElement && link.parentElement.nodeName === 'P') { - const div = document.createElement('div'); - link.parentElement.replaceWith(div); - div.appendChild(link); - } link.href = getFederatedUrl(link.href); return loadInlineFrags(link); }); + await Promise.all(fragPromises); } From dda67d539fdf283b37cf9cdd999374beb6ede0af Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Wed, 28 Feb 2024 16:34:48 +0200 Subject: [PATCH 3/9] added EOF --- test/blocks/fragment/mocks/fragments/frag-p.plain.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/blocks/fragment/mocks/fragments/frag-p.plain.html b/test/blocks/fragment/mocks/fragments/frag-p.plain.html index 2e360dbd86..6afc3df3e4 100644 --- a/test/blocks/fragment/mocks/fragments/frag-p.plain.html +++ b/test/blocks/fragment/mocks/fragments/frag-p.plain.html @@ -4,4 +4,4 @@
  • List item
  • List item
  • -
    \ No newline at end of file +
    From ce6b3128bad7dfbcedb20c16a00783850dde3247 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Thu, 29 Feb 2024 08:05:32 +0200 Subject: [PATCH 4/9] hotfix --- libs/blocks/fragment/fragment.js | 3 ++- test/utils/utils.test.js | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libs/blocks/fragment/fragment.js b/libs/blocks/fragment/fragment.js index f87fb86c8c..b796a7183d 100644 --- a/libs/blocks/fragment/fragment.js +++ b/libs/blocks/fragment/fragment.js @@ -64,9 +64,10 @@ export default async function init(a) { let inline = false; if (a.parentElement && a.parentElement.nodeName === 'P') { + const children = a.parentElement.childNodes; const div = document.createElement('div'); a.parentElement.replaceWith(div); - div.appendChild(a); + div.append(...children); } if (a.href.includes('#_inline')) { diff --git a/test/utils/utils.test.js b/test/utils/utils.test.js index 548adc0066..84588323a6 100644 --- a/test/utils/utils.test.js +++ b/test/utils/utils.test.js @@ -97,14 +97,14 @@ describe('Utils', () => { it('Does not unwrap when sibling content present', () => { const fragments = document.querySelectorAll('.link-block.fragment'); utils.decorateAutoBlock(fragments[1]); - expect(fragments[1].parentElement.nodeName).to.equal('P'); + expect(fragments[1].parentElement.nodeName).to.equal('DIV'); expect(fragments[1].parentElement.textContent).to.contain('My sibling'); }); it('Does not unwrap when not in paragraph tag', () => { const fragments = document.querySelectorAll('.link-block.fragment'); utils.decorateAutoBlock(fragments[1]); - expect(fragments[1].parentElement.nodeName).to.equal('P'); + expect(fragments[1].parentElement.nodeName).to.equal('DIV'); expect(fragments[1].parentElement.textContent).to.contain('My sibling'); }); }); From 3ac4024d3ecbd2c7e0dcb78a9d2222547131664b Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Thu, 29 Feb 2024 08:10:53 +0200 Subject: [PATCH 5/9] removed empty line --- libs/blocks/global-navigation/utilities/utilities.js | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/blocks/global-navigation/utilities/utilities.js b/libs/blocks/global-navigation/utilities/utilities.js index 46aa01f4ea..ff86dedce6 100644 --- a/libs/blocks/global-navigation/utilities/utilities.js +++ b/libs/blocks/global-navigation/utilities/utilities.js @@ -298,7 +298,6 @@ export async function fetchAndProcessPlainHtml({ url, shouldDecorateLinks = true link.href = getFederatedUrl(link.href); return loadInlineFrags(link); }); - await Promise.all(fragPromises); } From c8243a406a43d2e12fcf1e33a6f87bd5db4c592a Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Fri, 1 Mar 2024 07:49:09 +0200 Subject: [PATCH 6/9] hotfix --- libs/blocks/fragment/fragment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/blocks/fragment/fragment.js b/libs/blocks/fragment/fragment.js index b796a7183d..5542265e1f 100644 --- a/libs/blocks/fragment/fragment.js +++ b/libs/blocks/fragment/fragment.js @@ -63,7 +63,7 @@ export default async function init(a) { let relHref = localizeLink(a.href); let inline = false; - if (a.parentElement && a.parentElement.nodeName === 'P') { + if (a.parentElement?.nodeName === 'P') { const children = a.parentElement.childNodes; const div = document.createElement('div'); a.parentElement.replaceWith(div); From 2584d04ae073359453f119e3656e50834bea8804 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Fri, 1 Mar 2024 09:58:21 +0200 Subject: [PATCH 7/9] transfer attributes on replacing fragment parent --- libs/blocks/fragment/fragment.js | 1 + test/blocks/fragment/fragment.test.js | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/libs/blocks/fragment/fragment.js b/libs/blocks/fragment/fragment.js index 5542265e1f..9e4ef4e00d 100644 --- a/libs/blocks/fragment/fragment.js +++ b/libs/blocks/fragment/fragment.js @@ -66,6 +66,7 @@ export default async function init(a) { if (a.parentElement?.nodeName === 'P') { const children = a.parentElement.childNodes; const div = document.createElement('div'); + for (const attr of a.parentElement.attributes) div.setAttribute(attr.name, attr.value); a.parentElement.replaceWith(div); div.append(...children); } diff --git a/test/blocks/fragment/fragment.test.js b/test/blocks/fragment/fragment.test.js index 842be015ad..e2001033e8 100644 --- a/test/blocks/fragment/fragment.test.js +++ b/test/blocks/fragment/fragment.test.js @@ -86,4 +86,15 @@ describe('Fragments', () => { body.innerHTML = body.innerHTML; // after reassignment, the parser guarantees the presence of only valid HTML expect(innerHtml).to.equal(body.innerHTML); }); + + it('should transfer all attributes when replacing a paragraph parent with a div parent', async () => { + const a = document.querySelector('a.frag-p'); + const testAttr = Object.entries({ class: 'frag-p-wrapper', test: 'test' }); + for (const [name, value] of testAttr) a.parentElement.setAttribute(name, value); + await getFragment(a); + const wrapper = document.querySelector('.frag-p-wrapper'); + for (const [name, value] of testAttr) { + expect(wrapper.getAttribute(name)).to.equal(value); + } + }); }); From 6ee176d4ee3d5e32c2290c42a9e729d74e00b814 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Fri, 1 Mar 2024 10:04:39 +0200 Subject: [PATCH 8/9] refactor --- test/blocks/fragment/fragment.test.js | 7 +++---- test/blocks/fragment/mocks/body.html | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/test/blocks/fragment/fragment.test.js b/test/blocks/fragment/fragment.test.js index e2001033e8..7992d9e1d5 100644 --- a/test/blocks/fragment/fragment.test.js +++ b/test/blocks/fragment/fragment.test.js @@ -89,12 +89,11 @@ describe('Fragments', () => { it('should transfer all attributes when replacing a paragraph parent with a div parent', async () => { const a = document.querySelector('a.frag-p'); - const testAttr = Object.entries({ class: 'frag-p-wrapper', test: 'test' }); - for (const [name, value] of testAttr) a.parentElement.setAttribute(name, value); + const { attributes } = a.parentElement; await getFragment(a); const wrapper = document.querySelector('.frag-p-wrapper'); - for (const [name, value] of testAttr) { - expect(wrapper.getAttribute(name)).to.equal(value); + for (const attr of attributes) { + expect(wrapper.getAttribute(attr.name)).to.equal(attr.value); } }); }); diff --git a/test/blocks/fragment/mocks/body.html b/test/blocks/fragment/mocks/body.html index 9a89357518..835dc0e8ee 100644 --- a/test/blocks/fragment/mocks/body.html +++ b/test/blocks/fragment/mocks/body.html @@ -5,7 +5,7 @@ Fragment Fragment Fragment -

    +

    Fragment

    From 26f45edeb3510d8203fef2da6278d47dd312313d Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Mon, 4 Mar 2024 11:17:00 +0200 Subject: [PATCH 9/9] hotfix --- libs/blocks/fragment/fragment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/blocks/fragment/fragment.js b/libs/blocks/fragment/fragment.js index 9e4ef4e00d..f666a068e7 100644 --- a/libs/blocks/fragment/fragment.js +++ b/libs/blocks/fragment/fragment.js @@ -65,7 +65,7 @@ export default async function init(a) { if (a.parentElement?.nodeName === 'P') { const children = a.parentElement.childNodes; - const div = document.createElement('div'); + const div = createTag('div'); for (const attr of a.parentElement.attributes) div.setAttribute(attr.name, attr.value); a.parentElement.replaceWith(div); div.append(...children);