1720551 - [wayland] popup is wrongly anchored in RTL

Status: RESOLVED FIXED

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

Description

[sarmad]

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

Steps to reproduce:

• Start firefox in Arabic (right to left layout)
• Click the Application Menu button, or on a button of an extension that displays a menu.

Actual results:

Menu is not displayed. Sometimes the whole Firefox crashes.

Additional Details:

• It worked for a while, but once it started crashing it became 100% reproducible.
• This was tested on a fresh installation of Ubuntu 21.04, as well as a non-fresh installation of Pop OS 21.04, in Wayland session for both.
• Removing the entire ~/.mozilla folder does not help. Reinstalling firefox also does not help.
• Switching the language to English by simply running LANG=en firefox solves the issue.
• Switching to X11 seems to fix the issue as well.

Expected results:

Should work and display the menu correctly.

Comment 1

[Andre Klapper]

Please follow the steps on https://support.mozilla.org/en-US/kb/firefox-crashes-troubleshoot-prevent-and-get-help#w_get-help-fixing-this-crash and provide your Report ID here. Thanks!

Comment 2

[sarmad]

Here is the report id: bp-73c656c1-1970-4022-b18e-efc8b0210714

Comment 3

[sarmad]

Here is a bit more details I discovered:

• The issue of menu not showing up seems to be a Gnome/Wayland issue. The same behaviour happens in other GTK based apps, like Gnome Terminal's pop up menu.
• The crash isn't 100% reproducible with the missing menu issue. The crash happened few times after I clicked the menu, but I'm not 100% sure the crash is a result of the same issue.
• The missing menu seems to be related to the position of the menu on the screen, and to multi monitor setups. Here is how to reproduce the issue (again, this can also be reproduced in other apps):
  • Use Ubuntu 21.04 or Pop OS 21.04
  • Login using Wayland session.
  • Set the language to Arabic (it's probably reproducible in other RTL languages as well)
  • Attach a secondary monitor.
  • Put the Firefox window on a monitor that isn't the left most.
  • Either maximize the Firefox window, or move it close to the left edge of the monitor.
  • Click the main menu button.

Comment 4

[Petruta Horea [:phorea], Desktop Test Engineering]

Confirming based on crash-stats reports.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Per comment 0 it seems this is specific to wayland.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

I can try to repro this somehow.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

So I can't repro the crash on Nightly, but the popup is busted, because it's anchored to the left, outside of the window. Screenshots incoming.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: X11

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: wayland

The wayland behavior looks specially broken because if there's no enough space to the right, the popup doesn't appear at all. Martin, is this known?

Comment 10

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

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

Created attachment 9233956 [details]
wayland

The wayland behavior looks specially broken because if there's no enough space to the right, the popup doesn't appear at all. Martin, is this known?

Missing popup is a Mutter bug - https://gitlab.gnome.org/GNOME/mutter/-/issues/1768

Wrong anchor needs to be fixed.

Comment 11

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Emilio, how can I enable the RTL popup direction in nightly (custom builds)?
Thanks.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

intl.l10n.pseudo=bidi

Comment 13

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

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

intl.l10n.pseudo=bidi

Awesome, Thanks!

Comment 14

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Looks like we need to export IsDirectionRTL() from PopupFrame and adjust move-to-rect setup for it. All popup are placed as LTR by default.

Comment 15

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Gtk uses:

  if (text_direction == GTK_TEXT_DIR_RTL)
    {
      rect_anchor = get_horizontally_flipped_anchor (rect_anchor);
      menu_anchor = get_horizontally_flipped_anchor (menu_anchor);
    }

so we need to something similar for RTL locales.

Comment 16

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Attachment: Bug 1720551 [Wayland] Update popup anchor for RTL locales, r?jhorak

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b346c50a44c
[Wayland] Update popup anchor for RTL locales, r=emilio

Comment 18

[Andreea Pavel [:apavel]]

Backed out for bc failures on browser_test_clipboardcache.js failures

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=8b346c50a44c4a98b300bf528e4d3564dd167078&selectedTaskRun=P4GKso4xRCesjSc-VLSf3Q.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=348232556&repo=autoland&lineNumber=27862

Comment 19

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6a0ed223987
Backed out changeset 8b346c50a44c for causing bc failures on browser_test_clipboardcache.js. CLOSED TREE

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Are you sure this was causing those failures? This patch is Linux-only, and those failures are on Windows.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eacd6f08df5e
[Wayland] Update popup anchor for RTL locales, r=emilio

Comment 22

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/eacd6f08df5e

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Is this something we should keep on the radar for an eventual ESR91 backport?

Comment 24

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #23)

Is this something we should keep on the radar for an eventual ESR91 backport?

No need for that, Wayland should not be shipped for 91 ESR.