Matrix/element-web-Github
1
0
Fork 0
mirror of https://github.com/vector-im/element-web.git synced 2024-11-15 20:54:59 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fixes around URL tooltips and in-app matrix.to link handling (#9139)

Browse Source 
* Add regression test for tooltipify exposing raw HTML

* Handle m.to links involving children better

* Comments

* Fix mistaken assertion
This commit is contained in:
Michael Telatynski 2022-08-09 15:37:55 +01:00 committed by GitHub
parent 48ae16b5a5
commit e63072e21f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
cypress/e2e/regression-tests/pills-click-in-app.spec.ts
View File

@ -59,8 +59,9 @@ describe("Pills", () => {
// find the pill in the timeline and click it
cy.get(".mx_EventTile_body .mx_Pill").click();
const localUrl = `/#/room/#${targetLocalpart}:`;
// verify we landed at a sane place
cy.url().should("contain", `/#/room/#${targetLocalpart}:`);
cy.url().should("contain", localUrl);
cy.wait(250); // let the room list settle
@ -69,7 +70,7 @@ describe("Pills", () => {
cy.get(".mx_EventTile_body .mx_Pill .mx_Pill_linkText")
.should("have.css", "pointer-events", "none")
.click({ force: true }); // force is to ensure we bypass pointer-events
cy.url().should("contain", `https://matrix.to/#/#${targetLocalpart}:`);
cy.url().should("contain", localUrl);
});
});
});

16
src/components/views/messages/TextualBody.tsx
View File

@ -432,11 +432,17 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
* to start with (e.g. pills, links in the content).
*/
private onBodyLinkClick = (e: MouseEvent): void => {
const target = e.target as Element;
if (target.nodeName !== "A" || target.classList.contains(linkifyOpts.className)) return;
const { href } = target as HTMLLinkElement;
const localHref = tryTransformPermalinkToLocalHref(href);
if (localHref !== href) {
let target = e.target as HTMLLinkElement;
// links processed by linkifyjs have their own handler so don't handle those here
if (target.classList.contains(linkifyOpts.className)) return;
if (target.nodeName !== "A") {
// Jump to parent as the `<a>` may contain children, e.g. an anchor wrapping an inline code section
target = target.closest<HTMLLinkElement>("a");
}
if (!target) return;
const localHref = tryTransformPermalinkToLocalHref(target.href);
if (localHref !== target.href) {
// it could be converted to a localHref -> therefore handle locally
e.preventDefault();
window.location.hash = localHref;

20
src/utils/tooltipify.tsx
View File

@ -39,9 +39,7 @@ export function tooltipifyLinks(rootNodes: ArrayLike<Element>, ignoredNodes: Ele
let node = rootNodes[0];
while (node) {
let tooltipified = false;
if (ignoredNodes.indexOf(node) >= 0) {
if (ignoredNodes.includes(node) || containers.includes(node)) {
node = node.nextSibling as Element;
continue;
}
@ -49,20 +47,18 @@ export function tooltipifyLinks(rootNodes: ArrayLike<Element>, ignoredNodes: Ele
if (node.tagName === "A" && node.getAttribute("href")
&& node.getAttribute("href") !== node.textContent.trim()
) {
const container = document.createElement("span");
const href = node.getAttribute("href");
// The node's innerHTML was already sanitized before being rendered in the first place, here we are just
// wrapping the link with the LinkWithTooltip component, keeping the same children. Ideally we'd do this
// without the superfluous span but this is not something React trivially supports at this time.
const tooltip = <LinkWithTooltip tooltip={new URL(href, window.location.href).toString()}>
{ node.innerHTML }
<span dangerouslySetInnerHTML={{ __html: node.innerHTML }} />
</LinkWithTooltip>;
ReactDOM.render(tooltip, container);
node.replaceChildren(container);
containers.push(container);
tooltipified = true;
}
if (node.childNodes?.length && !tooltipified) {
ReactDOM.render(tooltip, node);
containers.push(node);
} else if (node.childNodes?.length) {
tooltipifyLinks(node.childNodes as NodeListOf<Element>, ignoredNodes, containers);
}

15
test/utils/tooltipify-test.tsx
View File

@ -57,4 +57,19 @@ describe('tooltipify', () => {
expect(containers).toHaveLength(0);
expect(root.outerHTML).toEqual(originalHtml);
});
it("does not re-wrap if called multiple times", () => {
const component = mount(<div><a href="/foo">click</a></div>);
const root = component.getDOMNode();
const containers: Element[] = [];
tooltipifyLinks([root], [], containers);
tooltipifyLinks([root], [], containers);
tooltipifyLinks([root], [], containers);
tooltipifyLinks([root], [], containers);
expect(containers).toHaveLength(1);
const anchor = root.querySelector("a");
expect(anchor?.getAttribute("href")).toEqual("/foo");
const tooltip = anchor.querySelector(".mx_TextWithTooltip_target");
expect(tooltip).toBeDefined();
});
});