Long hangs (75s) seen when opening the address bar due to waiting for the clipboard
Categories
(Firefox :: Address Bar, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
relnote-firefox | --- | 126+ |
firefox-esr115 | --- | unaffected |
firefox125 | + | wontfix |
firefox126 | + | verified disabled |
firefox127 | + | verified |
People
(Reporter: standard8, Assigned: klubana)
References
(Regression, )
Details
(Keywords: hang, regression, Whiteboard: [sng][search-performance])
Attachments
(2 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-beta+
|
Details | Review |
In Firefox 125 we enabled a new feature on release (bug 1882478) to present a suggestion on the address bar if the user has a url on their clipboard.
We have had a couple of reports on Reddit of users seeing long hangs when clicking on the address bar.
One user has been able to capture a profile, which shows a 75 second hang with this stack:
ZwUserGetClipboardData [win32u.dll]
GetClipboardData [user32.dll]
CClipDataObject::OleGetClipboardData(unsigned int, void**) [com\ole32\ole232\clipbrd\clipdata.cpp]
CClipDataObject::GetData(tagFORMATETC*, tagSTGMEDIUM*) [com\ole32\ole232\clipbrd\clipdata.cpp]
RepeatedlyTryGetData::<lambda_19>::operator()() const [widget/windows/nsClipboard.cpp]
RepeatedlyTry(RepeatedlyTryGetData::<lambda_19>, std::_Binder<std::_Unforced,void (&)(long, const nsTString<char> &),const std::_Ph<1> &,nsTLiteralString<char> >) [widget/windows/nsClipboard.cpp]
RepeatedlyTryGetData(IDataObject&, tagFORMATETC*, tagSTGMEDIUM*) [widget/windows/nsClipboard.cpp]
nsClipboard::FillSTGMedium(IDataObject*, unsigned int, tagFORMATETC*, tagSTGMEDIUM*, unsigned long) [widget/windows/nsClipboard.cpp]
nsClipboard::GetNativeDataOffClipboard(IDataObject*, unsigned int, unsigned int, char const*, void**, unsigned int*) [widget/windows/nsClipboard.cpp]
nsClipboard::GetDataFromDataObject(IDataObject*, unsigned int, nsIWidget*, nsITransferable*) [widget/windows/nsClipboard.cpp]
nsClipboard::GetNativeClipboardData(nsITransferable*, int) [widget/windows/nsClipboard.cpp]
nsBaseClipboard::GetData(nsITransferable*, int, mozilla::dom::WindowContext*) [widget/nsBaseClipboard.cpp]
...
readFromClipboard [chrome://browser/content/browser.js:3091:27]
isActive [resource:///modules/UrlbarProviderClipboard.sys.mjs:49:11]
js::RunScript
tryMethod [resource:///modules/UrlbarUtils.sys.mjs:2365:12]
start [resource:///modules/UrlbarProvidersManager.sys.mjs:405:14]
startQuery [resource:///modules/UrlbarProvidersManager.sys.mjs:215:19]
Comment 1•5 months ago
|
||
Let's disable the clipboard feature by default, users can still flip it on from Preferences if they wish.
It sounds like we really need bug 1874203.
Updated•5 months ago
|
Updated•5 months ago
|
Updated•5 months ago
|
Updated•5 months ago
|
Assignee | ||
Comment 2•5 months ago
|
||
Updated•5 months ago
|
Assignee | ||
Updated•5 months ago
|
Comment 4•5 months ago
|
||
For Firefox 126, Donal is likely already adding this, just adding it here for tracking purposes.
Release Note Request (optional, but appreciated)
[Why is this notable]: Disabling a previously released and rel-noted feature
[Affects Firefox for Android]: no
[Suggested wording]: The Clipboard Suggestion feature was temporarily disabled while the team investigates a potential performance issue. The feature will be re-enabled in a future release once the performance issue is addressed.
[Links (documentation, blog post, etc)]:
Assignee | ||
Comment 5•5 months ago
|
||
Original Revision: https://phabricator.services.mozilla.com/D209257
Updated•5 months ago
|
Comment 6•5 months ago
|
||
beta Uplift Approval Request
- User impact if declined: performance issue
- Code covered by automated testing: yes
- Fix verified in Nightly: no
- Needs manual QE test: yes
- Steps to reproduce for manual QE testing: Check the feature is properly disabled (no checkbox in about:preferences, no result in the address bar)
- Risk associated with taking this patch: Low
- Explanation of risk level: We're flipping of the feature gate preference, so returning to the status before the feature was released in 125
- String changes made/needed: None
- Is Android affected?: no
Comment 7•5 months ago
|
||
bugherder |
Updated•5 months ago
|
Updated•5 months ago
|
Comment 9•5 months ago
|
||
We have async clipboard APIs, is there any reason why navigator.clipboard.readText() is not usable?
Comment 10•5 months ago
•
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #9)
We have async clipboard APIs, is there any reason why navigator.clipboard.readText() is not usable?
https://bugzilla.mozilla.org/show_bug.cgi?id=1894718
Even if ideally we'd like to be able to peek at the clipboard size even before actually fetching it, or only fetch if size is less than N. But that capability is not available (https://bugzilla.mozilla.org/show_bug.cgi?id=1874203).
Updated•5 months ago
|
Comment 11•5 months ago
|
||
On SUMO, a Windows user reported only experiencing the problem when they have a remote desktop session open on a Linux host. Not sure whether that is relevant. https://support.mozilla.org/questions/1446274
Comment 12•5 months ago
•
|
||
Pref'd off of the feature update verified with 127.0a1 (2024-05-06) /126.0 (64-bit) RC on Windows 10/ Mac13.
Additionally, we have tried to reproduce the issue with 125.0 , 125.0.1, 125.0.2, 125.03 on Windows 11 Version 21h2 22H2 (OS Build 22000.1817) and Windows 11 Version 22H2 (OS Build 22621.2861) with no success. Seems that there are additional prerequisites to hit this.
Comment 13•5 months ago
|
||
Updating flags since this issue is not actually fixed, disabled flags for the 126/127 would make more sense?
Comment 14•5 months ago
|
||
(In reply to jscher2000 from comment #11)
On SUMO, a Windows user reported only experiencing the problem when they have a remote desktop session open on a Linux host. Not sure whether that is relevant. https://support.mozilla.org/questions/1446274
It may be relevant, depending on how remote desktop interacts with OLE clipboard. I have a small exe that when executed misuses the clipboard causing Firefox (or any other app using OLE clipboard) to hang.
Updated•5 months ago
|
Comment 15•5 months ago
|
||
Another SUMO user who has remote desktop with clipboard enabled added that after disabling clipboard suggestions, pasting into the address bar triggers the same problem. https://support.mozilla.org/en-US/questions/1446876#answer-1652076
Presumably there is something special about parsing a paste into the address bar which doesn't affect other fields. Is there a bug open for that? I couldn't find one, but my bug search skills are not very good.
Comment 16•5 months ago
•
|
||
(In reply to jscher2000 from comment #15)
Another SUMO user who has remote desktop with clipboard enabled added that after disabling clipboard suggestions, pasting into the address bar triggers the same problem. https://support.mozilla.org/en-US/questions/1446876#answer-1652076
First thing to note here, is that on Windows any third party app can lock any other app using the OLE clipboard when it tries to access the clipboard. I demonstrated it locally with few lines of code. That's not really fixable on our side, it's a Windows problem.
The only thing the urlbar does differently from other input fields is parsing the string to check if it's a URL, so of course it may have performance problems with very large strings. The user said it was a short string so it was not that problem. Then I suspect a locked clipboard as I explained, but that same issue was there in any previous versions of Firefox then.
Comment 17•5 months ago
|
||
Set release status flags based on info from the regressing bug 1882478
Comment 18•5 months ago
•
|
||
Bad bot - this was already fixed (by preffing off the feature) on nightly in the 127 cycle.
(ni=suhaib to see if we can figure out what happened in comment 17 and prevent it in other bugs. Thanks!)
Updated•5 months ago
|
Comment 19•5 months ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #18)
Bad bot - this was already fixed (by preffing off the feature) on nightly in the 127 cycle.
(ni=suhaib to see if we can figure out what happened in comment 17 and prevent it in other bugs. Thanks!)
Since the regression affected 125, all newer versions will be marked as affected until a version is marked as fixed
or verified
. I think this is what happened in comment 17. What do you think the bot should do differently?
Comment 20•5 months ago
•
|
||
Thanks. I guess the upshot of the bot issue is that we should never set the trunk status as disabled
, when we fix a bug by preffing off a feature on trunk indefinitely (i.e. not in a way where version numbers are relevant). Otherwise, the change from affected-to-disabled makes the bot implicitly think the bug will still affect future versions.
I filed https://github.com/mozilla/bugbot/issues/2403 on seeing if we can make the bot handle this more gracefully/helpfully.
Comment 21•5 months ago
|
||
Removing qe+ for now, please add it back when we have a patch here.
Description
•