Closed Bug 1901769 Opened 5 months ago Closed 3 months ago

[wayland] Async clipboard "Paste" permission context menu appears in wrong position and flickers

Categories

(Core :: XUL, defect, P2)

Firefox 126
Desktop
Linux
defect

Tracking

()

VERIFIED FIXED
131 Branch
Tracking Status
firefox-esr115 --- disabled
firefox-esr128 131+ verified
firefox127 --- wontfix
firefox128 --- wontfix
firefox129 --- wontfix
firefox130 --- wontfix
firefox131 --- verified

People

(Reporter: danielhunterjacobs, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(12 files)

95.46 KB, video/webm
Details
86.17 KB, video/webm
Details
718.81 KB, video/webm
Details
1.02 MB, application/gzip
Details
25.21 KB, application/octet-stream
Details
67.46 KB, video/webm
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:126.0) Gecko/20100101 Firefox/126.0

Steps to reproduce:

Open Firefox 126 on Fedora Linux 39 running Gnome on wayland.
Open https://jsfiddle.net/zm490d6a/
Copy some text so something is is the clipboard.
Click inside the textbox.

Actual results:

No menu with the clickable item "Paste" appeared.

Expected results:

A menu with the clickable item "Paste" should have appeared, which when click pasted the text from the clipboard. This happens on Gnome on Xorg (x11), just not on Gnome (wayland)

Component: Untriaged → DOM: Copy & Paste and Drag & Drop
OS: Unspecified → Linux
Product: Firefox → Core
Hardware: Unspecified → Desktop

Behavior on wayland, where the bug occurs (regular GNOME)

Attachment #9406656 - Attachment description: Screencast from 2024-06-11 10-34-45.webm → Behavior on wayland, where the bug occurs

Behavior on x11, where the bug does not occur (GNOME on Xorg)

Severity: -- → S3
Priority: -- → P2
Assignee: nobody → echen

Thanks for reporting this issue, I am not able to use wayland at the moment. Could you help to capture the log?

  • Open about:logging
  • Set log module to WidgetClipboard:5
  • Start the logging
  • Reproduce the issue
  • Stop the logging
  • Share the profiler link to us
Flags: needinfo?(danielhunterjacobs)
Flags: needinfo?(danielhunterjacobs)
Attached file Profiler

On my wayland, the paste contextmenu appears on the top-left corner of the browser window.
I used to see the paste button being rendered in right position, I think it is a regression.
And I got the following regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d3ddc98e7113bf3be63f35e439981aa3adb17e4d&tochange=b7f6557aaf06a26f589489fc439e74fd6e75a022

After reverting https://hg.mozilla.org/mozilla-central/rev/c6030081762c7e5285b255d4494c0368e4b920e0, the paste contextmenu is rendered in right position again.

Assignee: echen → nobody
Keywords: regression
Regressed by: 1777269
Component: DOM: Copy & Paste and Drag & Drop → Widget: Gtk

:stransky, could you take a look? Thanks!

Flags: needinfo?(stransky)

The async clipboard Paste context menu appears for me in the top-left corner of the window on Ubuntu 24.04 (Wayland).

Async clipboard was recently enabled for text in version 125 (Bug 1877400) (dom.events.asyncClipboard.readText) and other content in version 127 (Bug 1887845) (dom.events.asyncClipboard.clipboardItem).

Blocks: 1770358
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Clipboard readText menu not shown on wayland → [wayland] Async clipboard "Paste" permission context menu appears in wrong position and flickers

Looks like we're getting wrong popup position from layout:
https://searchfox.org/mozilla-central/rev/2f48061aef8c8976b73749ee845e7b85751f5f2f/widget/gtk/nsWindow.cpp#2408

[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]: nsWindow::WaylandPopupGetPositionFromLayout
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   layout popup CSS anchor (0, 0) (x=1560, y=1380, w=57600, h=61080), margin (t=240, r=240, b=240, l=240) offset (0,0)
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:     after margins (x=1320, y=1140, w=58080, h=61560)
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:     final (x=44, y=38, w=1936, h=2052)
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   relative popup rect position [44, 38] -> [1936 x 2052]

But the popup is not anchored so we may not use layout anchor rect but rather create our own one:

[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]: nsWindow::WaylandPopupHierarchyCalculatePositions()
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   popup [7fd70958ce00] set parent window [7fd732fe6a00]
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   popup [7fd70958ce00] bounds [783, 667] -> [63 x 24]
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   popup [7fd70958ce00] layout [1514, 1288] -> [126 x 48]
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   popup [7fd70958ce00] has toplevel as parent
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   popup [7fd70958ce00] transformed popup coordinates from [783, 667] to [783, 667]
[Parent 51917: Main Thread]: D/WidgetPopup [7fd70958ce00]:   popup [7fd70958ce00] matches layout [1] anchored [0] first popup [1] use move-to-rect 1

Emilio, I'm not very familiar with the layout code. Do you have an idea why popup layout anchor is different and do you think we may ignore it for non anchored popups?

Thanks.

Flags: needinfo?(emilio)

Edgar do you know why this code is doing something different to this which is what the context menu does?

I think the wayland code is getting confused with overlap, and I can dig into that, but it'd be great to know why that diverges from the regular context menu?

Flags: needinfo?(echen)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #14)

Edgar do you know why this code is doing something different to this which is what the context menu does?

I think the wayland code is getting confused with overlap, and I can dig into that, but it'd be great to know why that diverges from the regular context menu?

Good point. I will check that. I don't see a reason to do something different from the regular context menu.

Flags: needinfo?(stransky)
  • Remove unused after_pointer position.
  • Make mXPos/mYPos work in terms of margin (mExtraMargin).
  • When we have no anchor, explicitly upgrade them to anchored-to-point.
  • Make MenuPopupAnchorType an enum class.
Assignee: nobody → emilio
Status: NEW → ASSIGNED

I got some time to dig into this. The reason we get into the bad state is basically this.

The way we're handling this is rather complicated. Comment 16 substantially simplifies it in order to make Wayland automatically account for it.

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

I got some time to dig into this. The reason we get into the bad state is basically this.

The way we're handling this is rather complicated. Comment 16 substantially simplifies it in order to make Wayland automatically account for it.

Thank you!

Flags: needinfo?(echen)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/75398f72448b De indent some code in nsMenuPopupFrame::OpenPopup. r=layout-reviewers,jfkthame https://hg.mozilla.org/integration/autoland/rev/6dba8057201b Make MenuPopupAnchorType an enum class. r=layout-reviewers,jfkthame https://hg.mozilla.org/integration/autoland/rev/c5a4f119cfd1 Remove unused after_pointer popup position. r=layout-reviewers,jfkthame https://hg.mozilla.org/integration/autoland/rev/d05bacf3f608 Add some braces in nsMenuPopupFrame::MoveToAttributePosition. r=layout-reviewers,jfkthame
Keywords: leave-open
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4762fcad9e64 Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin
Flags: needinfo?(emilio)
Keywords: leave-open
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d5c4e504dfd6 Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin
Pushed by nerli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2eeba3d06bf9 Fix RTL offsets to be logical. r=fix https://hg.mozilla.org/integration/autoland/rev/bf75c1b988a0 Make browser_test_clipboard_contextmenu.js work with native macOS context menus. r=fix

Backed out for causing mochitest failures on test_bug477754.xhtml

[task 2024-08-05T15:12:03.709Z] 15:12:03     INFO - TEST-START | layout/xul/test/test_bug477754.xhtml
[task 2024-08-05T15:12:03.712Z] 15:12:03     INFO - GECKO(6184) | [Parent 6184, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /builds/worker/checkouts/gecko/chrome/nsChromeRegistry.cpp:182
[task 2024-08-05T15:12:03.713Z] 15:12:03     INFO - GECKO(6184) | [Parent 6184, Main Thread] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/chrome/nsChromeProtocolHandler.cpp:73
[task 2024-08-05T15:12:03.881Z] 15:12:03     INFO - GECKO(6184) | [WARN  webrender::renderer::init] asking to enable_gpu_markers but no supporting extension was found
[task 2024-08-05T15:12:04.048Z] 15:12:04     INFO - TEST-INFO | started process screentopng
[task 2024-08-05T15:12:04.330Z] 15:12:04     INFO - TEST-INFO | screentopng: exit 0
[task 2024-08-05T15:12:04.331Z] 15:12:04     INFO - TEST-UNEXPECTED-FAIL | layout/xul/test/test_bug477754.xhtml | RTL popup's right offset should be equal to the x offset passed to openPopup - got -10, expected 10
[task 2024-08-05T15:12:04.332Z] 15:12:04     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:509:14
[task 2024-08-05T15:12:04.333Z] 15:12:04     INFO - doTest@chrome://mochitests/content/chrome/layout/xul/test/test_bug477754.xhtml:44:9
[task 2024-08-05T15:12:04.334Z] 15:12:04     INFO - onpopupshown@chrome://mochitests/content/chrome/layout/xul/test/test_bug477754.xhtml:1:1
[task 2024-08-05T15:12:04.336Z] 15:12:04     INFO - GECKO(6184) | MEMORY STAT | vsize 11862MB | residentFast 529MB | heapAllocated 248MB
[task 2024-08-05T15:12:04.337Z] 15:12:04     INFO - GECKO(6184) | [Parent 6184, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /builds/worker/checkouts/gecko/chrome/nsChromeRegistry.cpp:182
[task 2024-08-05T15:12:04.339Z] 15:12:04     INFO - GECKO(6184) | [Parent 6184, Main Thread] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/chrome/nsChromeProtocolHandler.cpp:73
[task 2024-08-05T15:12:04.342Z] 15:12:04     INFO - GECKO(6184) | [Parent 6184, Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /builds/worker/checkouts/gecko/parser/html/nsHtml5StreamParser.cpp:1215
[task 2024-08-05T15:12:04.343Z] 15:12:04     INFO - TEST-OK | layout/xul/test/test_bug477754.xhtml | took 612ms
[task 2024-08-05T15:12:04.461Z] 15:12:04     INFO - TEST-START | layout/xul/test/test_bug703150.xhtml
Flags: needinfo?(emilio)
Attachment #9410727 - Attachment description: Bug 1901769 - Make nsMenuPopupFrame offset handling simpler. r=edgar,stransky → Bug 1901769 - Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0d53cc2130df Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 131 Branch

Backed out for causing bc failure on browser_navigator_clipboard_clickjacking.js

Backout link

Push with failures

Failure log

Status: RESOLVED → REOPENED
Flags: needinfo?(emilio)
Resolution: FIXED → ---
Target Milestone: 131 Branch → ---
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c4dd6ccf94ac Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin
Status: REOPENED → RESOLVED
Closed: 3 months ago3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 131 Branch

Backed out for causing mochitest failures on browser_navigator_clipboard_contextmenu_suppression.js

[task 2024-08-08T02:22:00.053Z] 02:22:00     INFO - TEST-START | dom/events/test/clipboard/browser_navigator_clipboard_contextmenu_suppression.js
[task 2024-08-08T02:22:17.932Z] 02:22:17     INFO - GECKO(1897) | 2024-08-08 02:22:17.931 firefox[1897:46346] Persistent UI failed to open file file:///Users/cltbld/Library/Saved%20Application%20State/org.mozilla.nightly.savedState/window_1.data: No such file or directory (2)
[task 2024-08-08T02:23:35.181Z] 02:23:35     INFO - TEST-INFO | started process screencapture
[task 2024-08-08T02:23:35.298Z] 02:23:35     INFO - TEST-INFO | screencapture: exit 0
[task 2024-08-08T02:23:35.298Z] 02:23:35     INFO - Buffered messages logged at 02:22:00
[task 2024-08-08T02:23:35.298Z] 02:23:35     INFO - Entering setup bound 
[task 2024-08-08T02:23:35.299Z] 02:23:35     INFO - Leaving setup bound 
[task 2024-08-08T02:23:35.299Z] 02:23:35     INFO - Entering test bound test_context_menu_suppression_sameorigin
[task 2024-08-08T02:23:35.300Z] 02:23:35     INFO - Console message: [JavaScript Warning: "The character encoding of a framed document was not declared. The document may appear different if viewed without the document framing it." {file: "https://example.com/browser/dom/events/test/clipboard/file_iframe.html" line: 0}]
[task 2024-08-08T02:23:35.300Z] 02:23:35     INFO - Console message: [JavaScript Warning: "Partitioned cookie or storage access was provided to “https://example.org/browser/dom/events/test/clipboard/file_iframe.html” because it is loaded in the third-party context and dynamic state partitioning is enabled."]
[task 2024-08-08T02:23:35.300Z] 02:23:35     INFO - Write data by clipboard.writeText()
[task 2024-08-08T02:23:35.301Z] 02:23:35     INFO - Test read from same-origin frame
[task 2024-08-08T02:23:35.301Z] 02:23:35     INFO - TEST-PASS | dom/events/test/clipboard/browser_navigator_clipboard_contextmenu_suppression.js | read should just be resolved without paste contextmenu shown - 
[task 2024-08-08T02:23:35.301Z] 02:23:35     INFO - Leaving test bound test_context_menu_suppression_sameorigin
[task 2024-08-08T02:23:35.302Z] 02:23:35     INFO - Entering test bound test_context_menu_suppression_crossorigin
[task 2024-08-08T02:23:35.302Z] 02:23:35     INFO - Console message: [JavaScript Warning: "The character encoding of a framed document was not declared. The document may appear different if viewed without the document framing it." {file: "https://example.com/browser/dom/events/test/clipboard/file_iframe.html" line: 0}]
[task 2024-08-08T02:23:35.303Z] 02:23:35     INFO - Console message: [JavaScript Warning: "Partitioned cookie or storage access was provided to “https://example.org/browser/dom/events/test/clipboard/file_iframe.html” because it is loaded in the third-party context and dynamic state partitioning is enabled."]
[task 2024-08-08T02:23:35.303Z] 02:23:35     INFO - Write data by clipboard.writeText()
[task 2024-08-08T02:23:35.303Z] 02:23:35     INFO - Test read from cross-origin frame
[task 2024-08-08T02:23:35.304Z] 02:23:35     INFO - Wait for paste button enabled
[task 2024-08-08T02:23:35.304Z] 02:23:35     INFO - Click paste button, request should be resolved
[task 2024-08-08T02:23:35.305Z] 02:23:35     INFO - Buffered messages logged at 02:22:50
[task 2024-08-08T02:23:35.305Z] 02:23:35     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 1
[task 2024-08-08T02:23:35.305Z] 02:23:35     INFO - Buffered messages finished
[task 2024-08-08T02:23:35.306Z] 02:23:35     INFO - TEST-UNEXPECTED-FAIL | dom/events/test/clipboard/browser_navigator_clipboard_contextmenu_suppression.js | Test timed out - 
[task 2024-08-08T02:23:35.306Z] 02:23:35     INFO - GECKO(1897) | Completed ShutdownLeaks collections in process 1897
[task 2024-08-08T02:23:35.307Z] 02:23:35     INFO - TEST-START | Shutdown
Status: RESOLVED → REOPENED
Flags: needinfo?(emilio)
Resolution: FIXED → ---
Target Milestone: 131 Branch → ---
Component: Widget: Gtk → XUL
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a73a333c97d7 Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/autoland/rev/f03e7d26ef79 Keep overlap position to make macOS test happy.
Status: REOPENED → RESOLVED
Closed: 3 months ago3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 131 Branch

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox130 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)
Regressions: 1912817

Comment on attachment 9416541 [details]
Bug 1901769 - Add some braces in nsMenuPopupFrame::MoveToAttributePosition. r=#layout

Beta/Release Uplift Approval Request

  • User impact if declined: Clipboard API on Linux Wayland doesn't work properly.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0
  • List of other uplifts needed: none
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): It's not the trivialest code, though the reason it bounced so much was mostly macOS tests due to using native context menus. Consider waiting a week or so before uplift perhaps?
  • String changes made/needed: none
  • Is Android affected?: Yes

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Breaks clipboard API on Linux.
  • User impact if declined: Clipboard API on Linux Wayland doesn't work properly.
  • Fix Landed on Version: 131
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): See above
Flags: needinfo?(emilio)
Attachment #9416541 - Flags: approval-mozilla-esr128?
Attachment #9416541 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Attachment #9416540 - Flags: approval-mozilla-esr128?
Attachment #9416540 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]

Verified the initial issue using Firefox 126 on Ubuntu 22.04 Wayland, with the paste button shown in the browser corner.
Verified that using latest Nightly 131.0a1 the paste button is shown where the cursor is when clicking inside the text area.

Comment on attachment 9416541 [details]
Bug 1901769 - Add some braces in nsMenuPopupFrame::MoveToAttributePosition. r=#layout

This is a long standing regression and we would have to land it at the end of the beta cycle for 130, let's have it ride the 131 train, thanks.

Attachment #9416541 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9416540 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Comment on attachment 9416540 [details]
Bug 1901769 - Remove unused after_pointer popup position. r=#layout

Approved for 128.3esr

Attachment #9416540 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
Attachment #9416541 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+

I used the latest Firefox 128 ESR build that does have the fix (https://treeherder.mozilla.org/jobs?repo=mozilla-esr128&revision=927837081d962926f7452d727eae04dfcfb6707e) and it does NOT look fixed to me, I can still reproduce the initial issue on Ubuntu 22.04 wayland.

Flags: needinfo?(emilio)

Well yeah that's because of course only two patches were requested for uplift, ugh, my fault :(

Flags: needinfo?(emilio)
Attachment #9410727 - Flags: approval-mozilla-esr128?
Attachment #9416538 - Flags: approval-mozilla-esr128?
Attachment #9416539 - Flags: approval-mozilla-esr128?

Comment on attachment 9410727 [details]
Bug 1901769 - Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin

Approved for 128.3esr

Attachment #9410727 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
Attachment #9416538 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
Attachment #9416539 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+

Backed out the last three patches for causing failures on nsMenuPopupFrame.cpp
Push with failures
Failure log
fyi there was a minor conflict with bug 1823552

Flags: needinfo?(emilio)
Attachment #9423554 - Flags: approval-mozilla-esr128?

Thanks, rebased it. It was just a matter of removing that line that was failing to build :)

Flags: needinfo?(emilio)

esr128 Uplift Approval Request

  • User impact if declined: See bug
  • Code covered by automated testing: yes
  • Fix verified in Nightly: yes
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: see bug
  • Risk associated with taking this patch: see bug
  • Explanation of risk level: see bug
  • String changes made/needed: none
  • Is Android affected?: no
Attachment #9423554 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+

Comment on attachment 9410727 [details]
Bug 1901769 - Make nsMenuPopupFrame offset handling simpler. r=edgar,NeilDeakin,TYLin

Approved on rebased patch D221481 instead

Attachment #9410727 - Flags: approval-mozilla-esr128+
Attachment #9416538 - Flags: approval-mozilla-esr128+
Attachment #9416539 - Flags: approval-mozilla-esr128+

Looks fixed to to me now using latest Firefox 128esr build on Ubuntu 22.04 wayland.

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

Attachment

General

Created:
Updated:
Size: