Closed Bug 1710507 Opened 4 years ago Closed 4 years ago

Native context menu on macOS doesn't support RTL

Categories

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

defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
thunderbird_esr78 --- unaffected
firefox-esr78 --- unaffected
firefox88 --- unaffected
firefox89 + verified
firefox90 + verified

People

(Reporter: itiel_yn8, Assigned: mstange)

References

(Regression)

Details

(Keywords: regression, rtl, Whiteboard: [proton-context-menus][mac:mr1] [priority:2b] [proton-uplift])

Attachments

(4 files)

Attached image Context menu displayed as LTR (deleted) —
No description provided.
Attached image RTL context menu (deleted) —

Screenshot was taken from VLC context menu, macOS version is 10.15.6.

Thanks for catching this! Fix is straightforward.

Assignee: nobody → mstange.moz
Status: NEW → ASSIGNED

[Tracking Requested - why for this release]: Regression for RTL users

Blocks: 34572
Whiteboard: [proton-context-menus][mac:mr1]
Priority: -- → P2
Whiteboard: [proton-context-menus][mac:mr1] → [proton-context-menus][mac:mr1] [priority:2b]

Does this also affect menubar menus? (Based on the code, it should.) Would menubar menus benefit from supporting RTL, too? Or would it be better to only fix context menus and leave menubar menus untouched?

Flags: needinfo?(itiel_yn8)
Attachment #9221211 - Attachment description: Bug 1710507 - Make native context menus respect the XUL element's directionality. r=emilio → Bug 1710507 - Make native menus respect the XUL element's directionality. r=emilio
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/694844621f2f Make native menus respect the XUL element's directionality. r=emilio

I've landed a patch which fixes this both for context menus and for menubar menus.

Chatting with the original reporter sh.yaron (cc'ed here), it makes sense to RTL the menubar as well.

Flags: needinfo?(itiel_yn8)

Thanks!

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

Is that needed for 89 or can this ride the 90 train?

Flags: needinfo?(mstange.moz)

This seems like a somewhat serious regression for RTL users, so I think it's worth uplifting to 89.

Flags: needinfo?(mstange.moz)

sh.yaron, the patch is in the latest Nightly. Could you check if things now behave the way you would expect? I'm especially interested in whether the menubar menus now look acceptable or if there is any odd-looking mixing of LTR and RTL directions.

Flags: needinfo?(sh.yaron)
Attached image Top bar still aligned to the left (deleted) —

This is the top bar after the fix with latest nightly.

The context menus look great, thank you.

Flags: needinfo?(sh.yaron)

The patch landed in nightly and beta is affected.
:mstange, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(mstange.moz)

Can we please uplift to Beta 89?

(In reply to Yaron Shahrabani from comment #14)

Created attachment 9221932 [details]
Top bar still aligned to the left

Thanks, I've filed bug 1711618 for that.

Comment on attachment 9221211 [details]
Bug 1710507 - Make native menus respect the XUL element's directionality. r=emilio

Beta/Release Uplift Approval Request

  • User impact if declined: In RTL locales, context menus will be displayed with left-to-right layout instead of right-to-left. This is a regression compared to non-native context menus.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • 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): Small fix, only affects RTL locales
  • String changes made/needed:
Flags: needinfo?(mstange.moz)
Attachment #9221211 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9221211 [details]
Bug 1710507 - Make native menus respect the XUL element's directionality. r=emilio

This is a major regression for RTL languages, approved for 89 beta 14, thanks.

Attachment #9221211 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]
Whiteboard: [proton-context-menus][mac:mr1] [priority:2b] → [proton-context-menus][mac:mr1] [priority:2b] [proton-uplift]

Reproduced using an affected Nightly build from 2021-05-10, verified that the context menu is displayed correctly from right to left on using ar and he build of latest Nightly, also using intl.l10n.pseudo - bidi.

Also verified as fixed using latest Firefox Beta 89.0b14 on macOS 11.3.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: