Unresponsiveness with 1Password extension #1919
Comments
victorlin
changed the title
|
I've run into this a couple of times this week already, and it's super annoying — have we had any update from the 1P folks? |
|
Nope, last I heard was to keep an eye on their releases page. I just checked the latest Beta (8.10.58.31) and Nightly (8.10.58.37) versions on the Chrome Web Store, and the problem still persists. I just sent a follow-up to their support team. |
|
This doesn't affect me personally or I might already be scratching this itch, but has anyone here looked into a) debugging this for them and/or b) developing active countermeasures we can use to disable the extension on Auspice pages? Both should be fully possible. |
My initial debugging on Slack showed that it's 
The long-running , Uo = class {
constructor() {
this.mutationObserver = new MutationObserver(t => {
for (let n of t) {
for (let r of n.addedNodes)
No(r) && zw(r) && this.intersectionObserver.observe(r);
for (let r of n.removedNodes)
No(r) && (this.intersectionObserver.unobserve(r),
this.resizeObserver.unobserve(r))
}
}
),
...
I don't think anyone has looked into this deeply besides the links that @jameshadfield shared initially indicating that it might not be possible. I just found a StackOverflow answer which indicates that it might be possible, but not yet tested. |
A couple thoughts from having used MutationObservers that make DOM modifications in an extension before.
|
|
Code snippets for each of the functions in the long-running call stack: 
(anonymous)this.mutationObserver = new MutationObserver(t => {
for (let n of t) {
for (let r of n.addedNodes)
No(r) && zw(r) && this.intersectionObserver.observe(r);
for (let r of n.removedNodes)
No(r) && (this.intersectionObserver.unobserve(r),
this.resizeObserver.unobserve(r))
}
}
),zwzw = e => {
let t = o => o.trim().replace(/[^a-zA-Z0-9]/g, "").toLowerCase()
, n = MM(e).map(t).filter(o => o !== "");
return IM.some(o => n.some(i => i.includes(o)))
}MMMM = e => {
let t = e instanceof HTMLImageElement ? [e.alt, e.className, e.id, e.title] : [e.id]
, n = RM(e);
return n !== null && t.push(n),
t
}RMvar LM = 4;
RM = e => {
if (e == null)
return null;
let t = e.parentNode
, n = 0;
for (; n < LM && t !== null; ) {
if (t instanceof HTMLElement && t.innerText && t.innerText.trim().length > 0)
return t.innerText;
t = t.parentNode,
n++
}
return null
} |
|
1Password support has narrowed the issue down to a specific feature. I've added it as a workaround in the issue description:
|
|
Ha, yeah, I'd also stumbled on that when digging into the source code. The MutationObserver in question is part of a The IM = [
"2fa",
"2stepverification",
"authenticationcode",
"authenticator",
"authy",
"barcodelogin",
"googleauthenticator",
"loginverification",
"microsoftauthenticator",
"multifactor",
"onetimepassword",
"qrcode",
"qrsignin",
"scancode",
"scantoconnect",
"securelogin",
"securitykey",
"temporaryaccesscode",
"timebasedcode",
"totp",
"twofactor",
"verifyidentity",
],The issue is that it doesn't just access the added element's I suspect putting it into a |
|
Oh! I think I have a workaround we can apply to Auspice. Let me test. |
|
Ah, ok, I can't test because I can't actually reproduce the issue locally. It appears that the one-time password autosaving feature isn't offered/available (on Linux? or something about my 1Password account?). There's not even a "Offer to save one-time passwords" setting. But, I think this patch in Auspice would do the trick (for the entropy panel). From 57b2f63e35ef8fd3dfea055ecaa36727d98701dc Mon Sep 17 00:00:00 2001
From: Thomas Sibley <tsibley@fredhutch.org>
Date: Thu, 16 Jan 2025 13:30:43 -0800
Subject: [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
large entropy panels, or really anytime thousands/millions of SVG
elements are added to the DOM.¹
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 our entropy panel's elements deeper than extension's
traversal limit of 4. That is, we're moving the closest HTMLElement
(the <div> containing the <svg>) further away from the entropy <rect>s.
Just one <g> would do, but adding four guards against future changes to
the internals of the #d3entropy element.
¹ <https://github.com/nextstrain/auspice/issues/1919>
---
src/components/entropy/index.js | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/components/entropy/index.js b/src/components/entropy/index.js
index 947996a8..715db041 100644
--- a/src/components/entropy/index.js
+++ b/src/components/entropy/index.js
@@ -290,7 +290,9 @@ class Entropy extends React.Component {
width={this.props.width}
height={this.props.height}
>
- <g ref={(c) => { this.d3entropy = c; }} id="d3entropy"/>
+ <g><g><g><g>
+ <g ref={(c) => { this.d3entropy = c; }} id="d3entropy"/>
+ </g></g></g></g>
</svg>
{this.resetLayout(styles)}
{this.entropyCountSwitch(styles)}
--
2.47.1And we could do something similar to other panels (tree, map, frequency) as needed too. |
|
Clever! I'll test the patch. |
|
It will only work if the bulk of the time is actually |
|
@victorlin Did you (or could you) save the profile in the screenshots above to a file you could share? |
|
I just switched to 1Password Nightly (version 8.10.58.39) which confirms that the bulk of the time is 
The patch above works well. Trace-20250116T141329.json, screenshot: 
|
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 <tsibley@fredhutch.org>
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 <tsibley@fredhutch.org>
|
The workaround has been released in Auspice 2.62.0 which is now used in https://next.nextstrain.org and https://auspice.us. Keeping this issue open so that we can revert the workaround when 1Password releases a proper fix. |
I wouldn't hold your breath… I'd probably close this issue—the behaviour is fixed—and keep the conversation with 1Password going in the background on the support ticket. If you really want an issue to track it, then I'd make a new issue, e.g. "Revert 1Password |
|
Good suggestion. Closing this now that the workaround has made its way to https://nextstrain.org and social posts have been shared to inform users. |
|
Updating here since this issue has previous discussion of the source code. 1Password has added a fix in version 8.10.60.11, available through their nightly build. It looks like the fix is the addition of a condition (re-formatted with whitespace and the function name - No = e => e instanceof HTMLImageElement || e instanceof SVGElement || e instanceof HTMLCanvasElement
+ No = e => e instanceof HTMLImageElement || e instanceof SVGElement && e.nodeName === "svg" || e instanceof HTMLCanvasElementSeems reasonable – conditioning the slow OTP detection feature on |
|
You got to diffing the source before I did. :-) Agreed that seems like a good fix. I'm a little surprised they didn't do it this way instead: - No = e => e instanceof HTMLImageElement || e instanceof SVGElement || e instanceof HTMLCanvasElement
+ No = e => e instanceof HTMLImageElement || e instanceof SVGSVGElement || e instanceof HTMLCanvasElementbut either way seems fine. |
Recent versions of 1Password browser extension (8.10.56(.28), all browsers) causes Auspice views to be slow and in some cases seemingly unresponsive. The slowness scales with the number of elements rendered by Auspice. This means it is most noticeable for the following scenarios per-panel:
Example for reproducing the issue
Workarounds
Ordered from least to most work involved. Instructions written for Chrome – may vary for other browsers.
✅ Apply a workaround in Auspice
Tell affected users to disable the "Offer to save one-time passwords" toggle in 1Password extension settings > Autofill & save or use a different browser/profile without the extension installed.
Tell affected users to downgrade to an older version of 1Password.
8.10.52.25.Progress
Internal links
The text was updated successfully, but these errors were encountered: