This has been the case since the beginning of time: I1c657fd46a2b5be4f27aa508a5cc0d946d6b98a8

From what I remember:

• The SearchOverlay should not be persistent. Eg. navigating to #/search should not load it.
• However.. it should close when the user clicks a back button.
• However it should not show again if the user clicks forward or clicks a URL which contains #/search.
• If I do a search and then click back I should be taken to the article NOT the search interface.

Point 2 is why we update the hash fragment and it becomes inconsistent elsewhere.

@Nirzar / @alexhollender could you help us review this behaviour and define how this should behave?

confirmed that this is also an issue on mobile web

@Jdlrobson your comments above look good to me. I think that's a good description of the desired behavior.

Should I assign this to one of you?

Just FYI to whoever works on this 3 seems a little optimistic :)
We're basically wrestling with the address bar here and the overlay manager was not setup to work this way.

I suspect we'll need to explore using replaceState (and update mediawiki.router to support that)

I agree with @Jdlrobson that to get this working, replaceState seems like the appropriate solution.
While looking at this problem though, I noticed that visiting a page with the #/search url, like https://en.m.wikipedia.org/wiki/Barack_Obama#/search doesn't actually trigger the search overlay either. Seems like that should be fixed too?

In T189212#4063987, @Jdrewniak wrote:

I agree with @Jdlrobson that to get this working, replaceState seems like the appropriate solution.
While looking at this problem though, I noticed that visiting a page with the #/search url, like https://en.m.wikipedia.org/wiki/Barack_Obama#/search doesn't actually trigger the search overlay either. Seems like that should be fixed too?

Agreed - we can add that to the task and try to re-estimate today.

Per acceptance criteria search should not show in that circumstance.... ping @Alex @Nirzar

Change 420727 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] [WIP] Triggering search overlay through mediawiki.router

https://gerrit.wikimedia.org/r/420727

@Jdlrobson @Niedzielski — chatted with @Jdrewniak and was thinking of updating the acceptance criteria to the following (changes are in bold):

• If the address bar has #/search and I refresh the page, SearchOverlay will remain present
• If SearchOverlay is open and I click browser back button, hide the SearchOverlay and ensure #/search is removed from URL
• If SearchOverlay is open and I click X (close) button, hide the SearchOverlay and ensure #/search is removed from URL
• If I am on page A, I do a search, click a search result B and click back I should be taken to the article A without the search open
• If I am on page D, click back and land on page C, open the SearchOverlay, close the search overlay, click Forward, I should be taken to page D (not the SearchOverlay)
• If I click the search input, click browser back and click browser forward the SearchOverlay should show (unless there is an article forwards in the browser history, which should take priority)

Thoughts?

@alexhollender the behaviour that SearchOverlay shows when #/search is present sounds fine. One word of caution - if a user shares a page with #/search in the URL there may be no way to close the overlay... but this is the same for the editor (https://en.m.wikipedia.org/wiki/Barack_Obama#/editor/0) - this is currently a limitation we have to work with.

@Jdrewniak mediawiki.router currently has no support for replaceState but we can add it provided we write tests... (T189173)

@Jdlrobson thanks for reviewing. My hope is that if we're better about ensuring that #/search gets removed when the SearchOverlay is closed, it will be less common for people to share with #/search in the URL.

@Jdrewniak I went ahead and updated the acceptance criteria in the ticket.

If the address bar has #/search and I refresh the page, SearchOverlay will remain present

I'm trying out @Jdrewniak's changes and this seems like good functionality but it's unbecoming when the results are not preserved. It almost look like an error. Consider the following case:

1. User visits https://en.m.wikipedia.org/wiki/Barack_Obama.
2. User taps search.
3. User types "university" and some results are shown.
4. User presses back and sees that "University of Hawaii" is what they're looking for.
5. User presses forward to complete their search.

As a user, I would expect my previous query (and hopefully results) to be preserved so that all I'd have to type is "of hawaii". Instead, I see this rather confusingly empty screen and must retype the whole query:

I'm not sure what's in scope for this ticket as a bug fix vs feature, but I wanted to clarify that if we persist search in navigation state, we may also want to 1) persist at least the query 2) improve the empty input screen with a graphic of some kind.

Hi guys, it's hard to explain back button behaviour using words, so I made a short video explaining (to myself) what I thought the ideal back button behaviour was.
https://drive.google.com/file/d/1hb5nqxGCwUwgeMPidvsAqdwR36csWHme/view?usp=sharing

Basically my first instinct was to use history.back() to close the overlay, but I found out this has problems T102946.

I still think having the router trigger the overlay, and not the overlay trigger the router (as is currently) is the more correct solution (it also allows us to use the overlayManager), but having the overlay close the page when it's open on pageload is not acceptable either. So, idea: don't show the search overlay on pageload, even if the #/search hash is present.
Like this:
https://drive.google.com/file/d/1LrhEoAi2fvwgD2R_n-ITz0ECvkOZmG5U/view?usp=sharing

This behaviour is actually consistent with @alexhollender's initial idea for how the search overlay should work - the rationale being that in case someone shares a url with the search overlay open, the user opening that link would see the article, and not the overlay. This approach would prevent that by intentionally removing the search overlay on pageload.

@Niedzielski re:clearing search results - I agree, I added an clearResults call to my initial patch, but it turns out that if you just let the browser do its thing, it actually does maintain the state of the search results when you click the back & forward buttons.

Change 422118 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Registering SearchOverlay with OverlayManager

https://gerrit.wikimedia.org/r/422118

@alexhollender code is up on staging for you to test prior to any merging. Let us know what you think.

@Jdlrobson in terms of testing: is there any way to get search results to show?

As far as I can tell the original issue has been fixed 🙌

There was this criteria which I added at some point after we started working on this, which is perhaps a bit of a stretch-goal:

• If I am on page D, click back and land on page C, open the SearchOverlay, close the search overlay, click Forward, I should be taken to page D (not the SearchOverlay)

I don't think this is happening. The only downside I see here is that if I to to page A, then B, then C, then D, then use the back button to go < back C < back B < back A, then open the search overlay, I loose the ability to then use the forward button to go forward > B forward > C forward > D. But honestly I might just be down way too far in the weeds here, finding weird nuanced problems that aren't really problems...

@alexhollender I've fixed staging. Search results should show now!

@Jdlrobson looks good 👍

Change 420727 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Use OverlayManager for SearchOverlay

https://gerrit.wikimedia.org/r/420727

Change 422118 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Registering SearchOverlay with OverlayManager

https://gerrit.wikimedia.org/r/422118

Since it wasn't specified - please test this on the beta cluster.

Looks good on beta. Tried multiple devices and browsers (ios, android - chrome, firefox, safari) and did not see the overlay problem.