Closed Bug 1894614 Opened 1 month ago Closed 1 month ago

Long hangs (75s) seen when opening the address bar due to waiting for the clipboard

Categories

(Firefox :: Address Bar, defect, P1)

defect

Tracking

()

RESOLVED FIXED
127 Branch
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)

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]

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.

See Also: → 1874203
Severity: -- → S2
Component: DOM: Copy & Paste and Drag & Drop → Address Bar
Priority: -- → P1
Product: Core → Firefox
Whiteboard: [sng][search-performance]
Assignee: nobody → klubana
Status: NEW → ASSIGNED
See Also: → 1894718
Pushed by klubana@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e91d8e436dcc
Disable the clipboard suggestions in the address bar due to performance concerns. r=mak
Flags: qe-verify+

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)]:

relnote-firefox: --- → ?
Attachment #9400010 - Flags: approval-mozilla-beta?

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
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch
Attachment #9400010 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

We have async clipboard APIs, is there any reason why navigator.clipboard.readText() is not usable?

(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).

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

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.

Updating flags since this issue is not actually fixed, disabled flags for the 126/127 would make more sense?

(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.

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.

(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.

Set release status flags based on info from the regressing bug 1882478

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!)

Flags: needinfo?(smujahid)

(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?

Flags: needinfo?(smujahid)

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.

Removing qe+ for now, please add it back when we have a patch here.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: