Steps to reproduce:

1) Use desktop Safari
2) Use the regular Olivero search block that appears in the header
3) Type in a search term, and then use the mouse to click the button to submit the form.

Result:

The search form disappears and no search is performed.

Cause

The root cause of this issue is https://bugs.webkit.org/show_bug.cgi?id=229895, which states that within Safari buttons do not gain focus after they are clicked on.

Comments

mherchel created an issue. See original summary.

mherchel’s picture

Status: Active » Needs review
StatusFileSize
new3 KB
new2.96 KB
andy-blum’s picture

Status: Needs review » Needs work

This is a major problem, and IMO is 100% a bad implementation by the webkit team that causes non-standard behavior on the web. Unfortunately based on my research it seems unlikely to be fixed in webkit any time soon and there are a couple of issues remaining here.

  1. Because the search icon that opens the dropdown is a button element, clicking it with JS disabled on Safari does nothing.
  2. Because the fix is a JS-based solution, the issue persists when JS is disabled.

A potential solution would be to refactor the Search block to render <a href="/search">{{icon}}</a> and then re-render a button element over top of that and wiring up the dropdown functionality.

mherchel’s picture

Status: Needs work » Needs review

A potential solution would be to refactor the Search block to render {{icon}} and then re-render a button element over top of that and wiring up the dropdown functionality.

I really disagree with this. If we go this route we'll be generating a whole bunch of untestable code (since our test bots don't run in Safari). The closing of the search form is triggered by JS, so a JS solution is appropriate IMO.

Looking into this further, the search form isn't even available in Safari when JS is disabled because the :focus-within never gets activated when the user clicks on the button to open it. This is a separate issue though and much less critical.

andy-blum’s picture

Tabbing over the search icon opens the drop-down though

andy-blum’s picture

@mherchel a less radical alternative is to listen for 'pointerdown'. Those events fire prior to focus leaving the other elements, closing the dropdown.

This still doesn't solve the issue of the search box opening on tab-over and then not interacting at all on click, though.

andy-blum’s picture

Adding `tabindex="0"` to the form allows the form to "catch" the dropped focus before it falls out of whatever element has `:focus-within` enabled.

mherchel’s picture

Tabbing over the search icon opens the drop-down though

True. But clicking the button with the mouse does not. This is broken when JS is disabled in Safari. But this is a separate issue.

a less radical alternative is to listen for 'pointerdown'. Those events fire prior to focus leaving the other elements, closing the dropdown.

I don't see how that's better than the current solution.

Adding `tabindex="0"` to the form allows the form to "catch" the dropped focus before it falls out of whatever element has `:focus-within` enabled.

This will also create an additional focus stop for browsers that do not have this bug.

Does the current solution not work for you??

andy-blum’s picture

Following a call this morning we want to move forward with the javascript implementation first and then handle the no-js fallback in safari in a separate issue. As Mike put it "this issue with javascript disabled is not something my mom will run into", and I'm inclined to agree.

This comment deliberately does not move the issue to RTBC just yet.

andy-blum’s picture

Status: Needs review » Needs work

Looks like this still won't work as expected for two reasons:

1) A click event requires a mousedown+mouseup user interaction, but safari drops focus on mousedown. A fast click will keep the search bar open as expected, but if a user clicks slowly enough (outside the bounds of the text input) the mousedown event leads to focusout, triggering the search bar to close before the 'has-click' class is added

2) Once the 'has-click' class is added the only way for it to be removed is by using the close button. Clicking outside the search area leaves the search bar open

andy-blum’s picture

StatusFileSize
new984 bytes

New patch attached that fixes the issue in Safari with javascript, and IMO improves the interface in Firefox and Chrome as well.

I added `tabindex="-1"` and that seems to allow the form wrapper to "catch" focus once it leaves the input element. The only remaining issue is the button is unusable in safari w/o javascript

mherchel’s picture

Status: Needs work » Needs review
andy-blum’s picture

StatusFileSize
new984 bytes

Patch for 10.0.x attached

mherchel’s picture

Status: Needs review » Needs work

This works perfectly!

Can you update the patches with code comments indicating why we're adding this attribute (with a link to the webkit bug)?

andy-blum’s picture

Status: Needs work » Needs review
StatusFileSize
new1.19 KB
andy-blum’s picture

StatusFileSize
new1.21 KB

Changed comment wording for clarity and to not upset CSpell

mherchel’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -safari. JavaScript +safari JavaScript

Tests are passing for both 9.4.x and 10.0.x. This is good to go and a great solution. Thanks @andy-blum!

mherchel’s picture

Issue tags: -safari JavaScript +safari, +JavaScript
andy-blum’s picture

Follow up issue for the rarer safari-and-no-javascript combination #3271227: Olivero: Search toggle button doesn't work on click in Safari without JavaScript

  • lauriii committed 5dc5496 on 10.0.x
    Issue #3269716 by andy-blum, mherchel: Olivero: Search is unusable in...

  • lauriii committed 6e1c917 on 10.0.x
    Revert "Issue #3269716 by andy-blum, mherchel: Olivero: Search is...
  • lauriii committed cab778d on 10.0.x
    Issue #3269716 by andy-blum, mherchel: Olivero: Search is unusable in...

  • lauriii committed eac3614 on 9.4.x
    Issue #3269716 by andy-blum, mherchel: Olivero: Search is unusable in...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed cab778d and pushed to 10.0.x. Also cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.