Make WordPress Core

Opened 2 months ago

Last modified 2 days ago

#58912 accepted defect (bug)

Mobile: Admin menu unexpectedly closes with Safari

Reported by: afercia's profile afercia Owned by: joedolson's profile joedolson
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.2
Component: Administration Keywords: has-screenshots has-patch needs-testing
Focuses: ui, accessibility, javascript Cc:

Description

Turns out the change introduced in #53587 / [55326] makes the admin menu unusable with Safari.

My testing environment:
macOS 13.4.1
Safari 16.5.2 (18615.2.9.11.10)
VoiceOver 10 (869.24)

To reproduce:

  • Emulate a smaller screen by using the 'Responsive Design Mode' in the Safari 'Develop' menu.
  • Go for example to the posts list page in the admin.
  • Click the hamburger icon at the top of the page to expand the admin menu.
  • Click, for example, the menu item: Users
  • Expected: the menu item to expand and reveal its sub-items.
  • Actual: the whole admin menu closes. Nothing else happens.
  • Click on a menu item that doesn't have any sub-item e.g.: Comments.
  • Expected: the browser to navigate to the Comments page.
  • Actual: nothing happens.

Although this happens only with Safari, the inability to navigate through the admin pages seems to me a pretty serious issue.

The implementation in #53587 / [55326] relies on detecting whether the clicked menu link has focus and is contained within the menu. Safari is known to _not_ set focus on clicked elements or at least, it _may_ decide to not set focus on some kind of elements.

After some debugging, at this point of the code:

With Chrome:

focusIsInToggle is false 
focusIsInSidebar is true 
$( ':focus' )[0] returns the clicked link 
document.activeElement returns the clicked link 

With Safari:

focusIsInToggle is false 
focusIsInSidebar is false 
$( ':focus' )[0] returns undefined 
document.activeElement returns the body element 

This confirm Safari doesn't set focus on the clicked link and the scripts fails.

Worth noting #53587 was reopened at some point because a new ticket reported a similar problem with iOS Safari, see #54837. The bug seem nnot limited to mobile Safari though.

See attached animated GIF to illustrate the unexpected collapsing of the menu.

Attachments (2)

safari.gif (413.5 KB) - added by afercia 2 months ago.
58912.2.diff (1.4 KB) - added by joedolson 6 weeks ago.
Updated patch

Download all attachments as: .zip

Change History (20)

@afercia
2 months ago

#1 @afercia
2 months ago

For reference: in the Gutenberg editor there is an ad-hoc component to detect when focus is outside of an element. See useFocusOutside where focus on some elements gets 'normalized' to avoid this issue. Some eyes from the Editor team would probably help a lot to solve this bug and abstract a bit the code so that it can be reused.

This ticket was mentioned in PR #4917 on WordPress/wordpress-develop by @rajinsharwar.


2 months ago
#2

  • Keywords has-patch added

Using blur instead of focusout for fixing browser issues in Safari

Trac ticket: https://core.trac.wordpress.org/ticket/58912

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


2 months ago

#4 @joedolson
2 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 6.4
  • Owner set to joedolson
  • Status changed from new to accepted

@Travel_girl commented on PR #4917:


2 months ago
#5

I have tested the patch and it works much better now.

But I have noticed, that some menu items ( Tools - Import or Apperance) only opens after 2 or 3 times klicking.

I have tested on:

Mac OS 12.6.5
Safari 16.4.1

#6 @oglekler
6 weeks ago

  • Keywords changes-requested added

I tested patch with Windows 10 and Chrome 115, everything is working fine as was before, but I don't have Mac OS and Safari.

@rajinsharwar can you please check the issue reported in the previous comment.

#7 @rajinsharwar
6 weeks ago

I don't have MacOS either @oglekler :'/, I tested the patch in Chrome, and Firefox and it does seem to work. Can anyone with Mac devices please check on the issue reported on #comment:5?

#8 @joedolson
6 weeks ago

In my testing on Mac/Safari, it didn't appear that this patch worked; the close behavior didn't fire on losing focus at all. Looking at alternatives.

@joedolson
6 weeks ago

Updated patch

#9 @joedolson
6 weeks ago

Added an alternate option. I think that monitoring blur is flawed because it depends on something having focus, which isn't always the case.

I switched this to a click event, firing on any click event outside of the menu. This doesn't close the menu if you're using the keyboard and exit the menu, however.

#10 @oglekler
6 weeks ago

It looks like click and focusout can be combined, or it is possible to add additional listener for keyup check the element which got focus is outside the menu or focus is loosed entirely 🤔

This ticket was mentioned in Slack in #core by oglekler. View the logs.


5 weeks ago

This ticket was mentioned in Slack in #core by marybaum. View the logs.


5 weeks ago

#13 @marybaum
5 weeks ago

  • Keywords changes-requested removed

THis could use a little more testing before getting declared commit-ready.

#14 @rcorrales
5 weeks ago

I locally tested @joedolson patch in Safari last week, and it solved the issue. However, I cannot successfully navigate the menu with the keyboard, not even before the change, with or without the emulator on a smaller screen. In other browsers, when the menu is open, I can press the tab to go through the items, but on Safari, it shifts the focus to the next element (the Screen Options dropdown). The navigation in Safari could be a separate problem, though.

Last edited 5 weeks ago by rcorrales (previous) (diff)

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #core by ironprogrammer. View the logs.


11 days ago

#17 @binsaifullah
3 days ago

I have tested it with my iMac and found it is working fine now.

Mac OS Ventura 13.5.2
Safari 16.6

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


2 days ago

Note: See TracTickets for help on using tickets.