From 17b3f5c08af54c618de64d8673bd6adb5b16abcb Mon Sep 17 00:00:00 2001 From: Victor Lin <13424970+victorlin@users.noreply.github.com> Date: Fri, 17 Jan 2025 15:54:51 -0800 Subject: [PATCH] [PATCH] Workaround 1Password browser extension bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug pathologically slows down (and sometimes crashes) pages with thousands/millions of SVG elements in the DOM, which happens in reasonably sized datasets.¹ Short circuit the extension's calls to the very slow .innerText in its function: RM = (e) => { if (e == null) return null; let t = e.parentNode, n = 0; for (; n < 4 /*LM*/ && t !== null; ) { if ( t instanceof HTMLElement && t.innerText && t.innerText.trim().length > 0 ) return t.innerText; (t = t.parentNode), n++; } return null; }, by nesting each panel's D3 elements deeper than extension's traversal limit of 4. That is, we're moving the closest HTMLElement (e.g.
s containing s) further away from the leaf SVG elements. For most panels, fewer s would do, but adding four guards against future changes to D3 implementation. The workaround was trivial for entropy and frequencies panels which already have a separation where size/styles are applied on a parent and D3 operations happen on a child . It was still not too bad for map and measurements panels - those did not have a pre-existing separation, but adding the separation was trivial. The tree panel was the least trivial - the code and type annotations assumed that the root SVG element for D3 operations was an , so switching it to required updating those assumptions. Additionally, there is code that retrieves the width/height from the root SVG element, so it must be retained on the child element even when nested under an , which requires width and height to be set explicitly. ¹ Co-authored-by: Thomas Sibley --- src/components/entropy/index.js | 7 ++++++- src/components/frequencies/index.js | 7 ++++++- src/components/map/map.js | 5 ++++- src/components/measurements/measurementsD3.js | 7 ++++--- src/components/tree/phyloTree/change.ts | 4 ++-- src/components/tree/phyloTree/layouts.ts | 1 + src/components/tree/phyloTree/renderers.ts | 4 ++-- src/components/tree/phyloTree/types.ts | 2 +- src/components/tree/tree.tsx | 21 ++++++++++++++----- 9 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/components/entropy/index.js b/src/components/entropy/index.js index 947996a8e..ed100ac46 100644 --- a/src/components/entropy/index.js +++ b/src/components/entropy/index.js @@ -290,7 +290,12 @@ class Entropy extends React.Component { width={this.props.width} height={this.props.height} > - { this.d3entropy = c; }} id="d3entropy"/> + {/* TODO: remove intermediate s once the 1Password extension interference is resolved + * + */} + + { this.d3entropy = c; }} id="d3entropy"/> + {this.resetLayout(styles)} {this.entropyCountSwitch(styles)} diff --git a/src/components/frequencies/index.js b/src/components/frequencies/index.js index 7db4824d6..f1ed87cc8 100644 --- a/src/components/frequencies/index.js +++ b/src/components/frequencies/index.js @@ -126,7 +126,12 @@ class Frequencies extends React.Component { overflow: "visible" }} > - { this.domRef = c; }} id="d3frequencies"/> + {/* TODO: remove intermediate s once the 1Password extension interference is resolved + * + */} + + { this.domRef = c; }} id="d3frequencies"/> + ); diff --git a/src/components/map/map.js b/src/components/map/map.js index 76b199e00..ad7cecc14 100644 --- a/src/components/map/map.js +++ b/src/components/map/map.js @@ -176,7 +176,10 @@ class Map extends React.Component { this.state.responsive && !this.state.d3DOMNode ) { - const d3DOMNode = select("#map svg").attr("id", "d3DemesTransmissions"); + /* TODO: remove intermediate s once the 1Password extension interference is resolved + * + */ + const d3DOMNode = select("#map svg").attr("id", "d3DemesTransmissions").append("g").append("g").append("g").append("g"); this.setState({d3DOMNode}); } } diff --git a/src/components/measurements/measurementsD3.js b/src/components/measurements/measurementsD3.js index de4693ed5..1ac8adf69 100644 --- a/src/components/measurements/measurementsD3.js +++ b/src/components/measurements/measurementsD3.js @@ -195,12 +195,13 @@ export const drawMeasurementsSVG = (ref, xAxisRef, svgData) => { // Do not draw SVG if there are no measurements if (groupedMeasurements && groupedMeasurements.length === 0) return; - const svg = select(ref); - // The number of groups is the number of subplots, which determines the final SVG height const totalSubplotHeight = (layout.subplotHeight * groupedMeasurements.length); const svgHeight = totalSubplotHeight + layout.topPadding; - svg.attr("height", svgHeight); + /* TODO: remove intermediate s once the 1Password extension interference is resolved + * + */ + const svg = select(ref).attr("height", svgHeight).append("g").append("g").append("g").append("g"); // x-axis is in a different SVG element to allow sticky positioning drawStickyXAxis(xAxisRef, containerHeight, svgHeight, xScale, x_axis_label); diff --git a/src/components/tree/phyloTree/change.ts b/src/components/tree/phyloTree/change.ts index aa839b0c6..7fbabc0b7 100644 --- a/src/components/tree/phyloTree/change.ts +++ b/src/components/tree/phyloTree/change.ts @@ -75,7 +75,7 @@ const svgSetters = { }; -type UpdateCall = (selection: Transition) => void; +type UpdateCall = (selection: Transition) => void; /** createUpdateCall @@ -111,7 +111,7 @@ function createUpdateCall( } const genericSelectAndModify = ( - svg: Selection, + svg: Selection, treeElem: TreeElement, updateCall: UpdateCall, transitionTime: number, diff --git a/src/components/tree/phyloTree/layouts.ts b/src/components/tree/phyloTree/layouts.ts index b565d0767..9eedecc06 100644 --- a/src/components/tree/phyloTree/layouts.ts +++ b/src/components/tree/phyloTree/layouts.ts @@ -310,6 +310,7 @@ export const setScales = function setScales(this: PhyloTreeType): void { this.yScale = scaleLinear(); } + // TODO: access these from d3treeParent so they don't have to be set twice const width = parseInt(this.svg.attr("width"), 10); const height = parseInt(this.svg.attr("height"), 10); if (this.layout === "radial" || this.layout === "unrooted") { diff --git a/src/components/tree/phyloTree/renderers.ts b/src/components/tree/phyloTree/renderers.ts index fecaa7996..b0274cb28 100644 --- a/src/components/tree/phyloTree/renderers.ts +++ b/src/components/tree/phyloTree/renderers.ts @@ -28,8 +28,8 @@ export const render = function render( dateRange, scatterVariables }: { - /** the svg into which the tree is drawn */ - svg: Selection + /** the SVG element into which the tree is drawn */ + svg: Selection /** the layout to be used, e.g. "rect" */ layout: Layout diff --git a/src/components/tree/phyloTree/types.ts b/src/components/tree/phyloTree/types.ts index 0304e1fea..5f1e363e3 100644 --- a/src/components/tree/phyloTree/types.ts +++ b/src/components/tree/phyloTree/types.ts @@ -278,7 +278,7 @@ export interface PhyloTreeType { setScales: typeof layouts.setScales showTemporalSlice: typeof grid.showTemporalSlice strainToNode: Record - svg: Selection + svg: Selection timeLastRenderRequested?: number unrootedLayout: typeof layouts.unrootedLayout updateBranchLabels: typeof labels.updateBranchLabels diff --git a/src/components/tree/tree.tsx b/src/components/tree/tree.tsx index e4398230c..a7adf6902 100644 --- a/src/components/tree/tree.tsx +++ b/src/components/tree/tree.tsx @@ -27,8 +27,8 @@ const rhsTreeId = "RIGHT"; export class TreeComponent extends React.Component { domRefs: { - mainTree: SVGSVGElement | null; - secondTree: SVGSVGElement | null; + mainTree: SVGGElement | null; + secondTree: SVGGElement | null; }; tangleRef?: Tangle; clearSelectedNode: (node: SelectedNode) => void; @@ -186,12 +186,23 @@ export class TreeComponent extends React.Component {mainTree ? this.domRefs.mainTree = c : this.domRefs.secondTree = c;}} - /> + > + {/* TODO: remove intermediate s once the 1Password extension interference is resolved + * + */} + + {mainTree ? this.domRefs.mainTree = c : this.domRefs.secondTree = c;}} + /> + + ); }