1706624 - [Wayland] Don't create wl_seat but get it from Gtk

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[Martin Stránský [:stransky] (ni? me)]

Use wl_seat0 used by Gtk to avoid potential focus issues with our own wl_seat.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1706624 [Wayland] Use wl_seat0 from Gtk, r?jhorak

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c57f69f52ae
[Wayland] Use wl_seat0 from Gtk, r=jhorak

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/7c57f69f52ae

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 9217414 [details]
Bug 1706624 [Wayland] Use wl_seat0 from Gtk, r?jhorak

Beta/Release Uplift Approval Request

• User impact if declined: Broken Drag and Drop operations on Wayland.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch removes duplicated wl_seat and uses only one. Should not cause any issues.
• String changes made/needed: none

Comment 6

[Pascal Chevrel:pascalc]

Martin, is that a recent regression?

Comment 7

[Robert Mader [:rmader]]

I think this is meant to fix bug 1704287, which is a regression form bug 1703490, which landed in 89.

Comment 8

[Pascal Chevrel:pascalc]

Do we have the confirmation that this fix that just landed in nightly fixed the issue in bug 1704287? It seems that this could be easily manually tested but the uplift request doesn't ask for QE to verify the fix. I would prefer it to be verified on Nightly before taking it into beta. Thanks

Comment 9

[Martin Stránský [:stransky] (ni? me)]

The situation is not clear here as there are actually two issues with D&D:

1. There are two Wayland data_devices used by Firefox and only one data_device is used for D&D operation by Wayland compositor. That means D&D randomly works/fails which depends on witch device is used. This bug improves it that the two Firefox data devices uses the same wl_seat which reduces the compositor bug but does not fix that completely. From my experience is 100%-50% failing D&D operations without the patch here and 10% failing D&D operations with this patch. So this still needs to be addressed on Wayland compositor side.

2. Although D&D handlers are called the D&D operation is not finished correctly, i.e. not accepted/confirmed by Firefox. This happens in ~10% of cases and it's completely different bug.

So while this patch improves the situation dramatically there are still bugs at D&D which needs to be investigated.

(In reply to Robert Mader [:rmader] from comment #7)

I think this is meant to fix bug 1704287, which is a regression form bug 1703490, which landed in 89.

According to https://bugzilla.mozilla.org/show_bug.cgi?id=1704287#c10 this is not directly related to this one and seems to be case 2) as the D&D handlers are fired by compositor but are not correctly processed by Firefox.

Before Bug 1703490 there were two D&D handlers so the case 2) was a bit hidden, D&D worked as far as at least one D&D handler was handled correctly on Firefox side.

Comment 10

[Pascal Chevrel:pascalc]

Martin, after the backout of bug 1703490 will we still need to uplift this one? Thanks

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Pascal, we don't need to uplift this one, it's just a minor fix to save resources.
Thanks.

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9217414 [details]
Bug 1706624 [Wayland] Use wl_seat0 from Gtk, r?jhorak

Thanks

Comment 13

[Martin Stránský [:stransky] (ni? me)]

To be clear here: It's just a minor fix to save resources when bug 1703490 is backed out (i.e. we use two D&D handlers).