Setting popover content or template without using innerHTML

[jcorporation]

Is it an option for you to refactor the popover component to not use innerHTML?

Background is: I would implement a Trusted Types csp policy in my bootstrap.native based project without exceptions or sanitize library.

[thednp]

We might need an html option for Popover/Tooltip in that case. Easy to implement.

[jcorporation]

You mean an option to append a dom node? This would be greate!

[thednp]

I was thinking about a way to make it work without an option, but I'm not sure why the original plugins use the .append() method.

[jcorporation]

I think they accept a string or an element for the content of a tooltip/popover.

[thednp]

Yes we know that, but I mean what for? The reason I ask is because once initialized, Popper/Tooltip instances, their contents are pretty much static, they won't handle dynamic content.

[thednp]

After checking the append() in detail, I believe this one might be suitable in most situations, especially because BS5 dropped support for IE.

[thednp]

@jcorporation please test latest master and check how new code (src/util/setHtml.js) works.

[jcorporation]

Thanks for implementing this.

Code for setHtml looks good, but you use innerHTML to set the template. In my understanding the template.innerHTML could be replaced with setHtml also?

[thednp]

Ah that's a good idea, lemme do some tests.

[thednp]

Check master again :)

[jcorporation]

I have now checked with enabled trusted types csp policy:

1. using your template causes a trusted types error: DOMParser is not allowed. That is no issue - we can define a own template as a dom element.

2. using a self defined template as dom element causes a trusted types error: innerHTML is not allowed
   I had to change the following:

//    setHtml(popover, htmlMarkup.innerHTML);
    setHtml(popover, htmlMarkup);

3. I created the title with document.createTextNode to not use a string
   I had to change the following:

//if (content instanceof Element) { 
if (content instanceof Object) { 

instanceof detects the textNode not as an element.

Edit: it seems positioning the popover element does not work after the setHTML element change. But I have some issues with positioning and overflow before.

[thednp]

//    setHtml(popover, htmlMarkup.innerHTML);
    setHtml(popover, htmlMarkup);

This would create a popover inside a popover, which produces the positioning issue you're talking about. There has to be a way to be able to use innerHTML in this case.

[jcorporation]

Right, this is the problem.

We could iterate through the childNodes of htmlMarkup and add it one by one?

This snippet works for me:

Array.from(htmlMarkup.children).forEach((child) => {
  setHtml(popover, child);
});

[thednp]

That would not work with the following: "Some <span>text</span> element.", it will only add the span element.

[jcorporation]

Right, other idea:

• in line 101 you create the popover element with document.createElement
• if the template is a dom element we can clone it and we do not need copy the content from the template in the new created element

Following changes are make it work for me:

if template is a string use createElement else clone the domNode:

self.popover = template instanceof Element ? template.cloneNode(true) : document.createElement('div');

Load template only if it is a string - put the load template block in a if block

if (!template instanceof Element) {

Use if (content instanceof Object) instead of if (content instanceof Element) to respect domTextNodes in the setHtml function.

Eventually is this cloneNode approach also feasible for setting the other content settings and we can simplify the setHtml function?

[thednp]

The cloneNode(true) is very interesting, perhaps best in this case. However, the if (content instanceof Object) condition won't be good enough, it should throw an error when content is a JavaScript object (EG {a: 1, b: 2}).

Let me test the cloneNode(true) first. This might actually prove to be a great feature which would require proper documentation.

[thednp]

@jcorporation please test latest master and let me know how it works. Also know that your proposed changes also apply to Tooltip component.

[jcorporation]

It works now like a charm with enabled trusted types policy. Thanks for this great piece of software!