Skip to content

Commit

Permalink
[PATCH] Workaround 1Password browser extension bug
Browse files Browse the repository at this point in the history
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. <div>s
containing <svg>s) further away from the leaf SVG elements.

For most panels, fewer <g>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
<svg> and D3 operations happen on a child <g>. 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 <svg>, so
switching it to <g> 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
<svg>, which requires width and height to be set explicitly.

¹ <#1919>

Co-authored-by: Thomas Sibley <[email protected]>
  • Loading branch information
victorlin and tsibley committed Jan 21, 2025
1 parent a6076d5 commit 17b3f5c
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 16 deletions.
7 changes: 6 additions & 1 deletion src/components/entropy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,12 @@ class Entropy extends React.Component {
width={this.props.width}
height={this.props.height}
>
<g ref={(c) => { this.d3entropy = c; }} id="d3entropy"/>
{/* TODO: remove intermediate <g>s once the 1Password extension interference is resolved
* <https://github.com/nextstrain/auspice/issues/1919>
*/}
<g><g><g><g>
<g ref={(c) => { this.d3entropy = c; }} id="d3entropy"/>
</g></g></g></g>
</svg>
{this.resetLayout(styles)}
{this.entropyCountSwitch(styles)}
Expand Down
7 changes: 6 additions & 1 deletion src/components/frequencies/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,12 @@ class Frequencies extends React.Component {
overflow: "visible"
}}
>
<g ref={(c) => { this.domRef = c; }} id="d3frequencies"/>
{/* TODO: remove intermediate <g>s once the 1Password extension interference is resolved
* <https://github.com/nextstrain/auspice/issues/1919>
*/}
<g><g><g><g>
<g ref={(c) => { this.domRef = c; }} id="d3frequencies"/>
</g></g></g></g>
</svg>
</Card>
);
Expand Down
5 changes: 4 additions & 1 deletion src/components/map/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <g>s once the 1Password extension interference is resolved
* <https://github.com/nextstrain/auspice/issues/1919>
*/
const d3DOMNode = select("#map svg").attr("id", "d3DemesTransmissions").append("g").append("g").append("g").append("g");
this.setState({d3DOMNode});
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/components/measurements/measurementsD3.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <g>s once the 1Password extension interference is resolved
* <https://github.com/nextstrain/auspice/issues/1919>
*/
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);
Expand Down
4 changes: 2 additions & 2 deletions src/components/tree/phyloTree/change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const svgSetters = {
};


type UpdateCall = (selection: Transition<SVGGElement, PhyloNode, SVGSVGElement | null, unknown>) => void;
type UpdateCall = (selection: Transition<SVGGElement, PhyloNode, SVGGElement | null, unknown>) => void;


/** createUpdateCall
Expand Down Expand Up @@ -111,7 +111,7 @@ function createUpdateCall(
}

const genericSelectAndModify = (
svg: Selection<SVGSVGElement | null, unknown, null, unknown>,
svg: Selection<SVGGElement | null, unknown, null, unknown>,
treeElem: TreeElement,
updateCall: UpdateCall,
transitionTime: number,
Expand Down
1 change: 1 addition & 0 deletions src/components/tree/phyloTree/layouts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
4 changes: 2 additions & 2 deletions src/components/tree/phyloTree/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ export const render = function render(
dateRange,
scatterVariables
}: {
/** the svg into which the tree is drawn */
svg: Selection<SVGSVGElement | null, unknown, null, unknown>
/** the SVG element into which the tree is drawn */
svg: Selection<SVGGElement | null, unknown, null, unknown>

/** the layout to be used, e.g. "rect" */
layout: Layout
Expand Down
2 changes: 1 addition & 1 deletion src/components/tree/phyloTree/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export interface PhyloTreeType {
setScales: typeof layouts.setScales
showTemporalSlice: typeof grid.showTemporalSlice
strainToNode: Record<string, PhyloNode>
svg: Selection<SVGSVGElement | null, unknown, null, unknown>
svg: Selection<SVGGElement | null, unknown, null, unknown>
timeLastRenderRequested?: number
unrootedLayout: typeof layouts.unrootedLayout
updateBranchLabels: typeof labels.updateBranchLabels
Expand Down
21 changes: 16 additions & 5 deletions src/components/tree/tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const rhsTreeId = "RIGHT";
export class TreeComponent extends React.Component<TreeComponentProps, TreeComponentState> {

domRefs: {
mainTree: SVGSVGElement | null;
secondTree: SVGSVGElement | null;
mainTree: SVGGElement | null;
secondTree: SVGGElement | null;
};
tangleRef?: Tangle;
clearSelectedNode: (node: SelectedNode) => void;
Expand Down Expand Up @@ -186,12 +186,23 @@ export class TreeComponent extends React.Component<TreeComponentProps, TreeCompo
}) {
return (
<svg
id={mainTree ? "MainTree" : "SecondTree"}
id="d3treeParent"
style={{pointerEvents: "auto", cursor: "default", userSelect: "none"}}
width={width}
height={height}
ref={(c) => {mainTree ? this.domRefs.mainTree = c : this.domRefs.secondTree = c;}}
/>
>
{/* TODO: remove intermediate <g>s once the 1Password extension interference is resolved
* <https://github.com/nextstrain/auspice/issues/1919>
*/}
<g><g><g><g>
<g
id={mainTree ? "MainTree" : "SecondTree"}
width={width}
height={height}
ref={(c) => {mainTree ? this.domRefs.mainTree = c : this.domRefs.secondTree = c;}}
/>
</g></g></g></g>
</svg>
);
}

Expand Down

0 comments on commit 17b3f5c

Please sign in to comment.