virtualstate / navigation

Native JavaScript navigation [web api] implementation
MIT License
77 stars 5 forks source link

[firefox] Navigation event not fired when traversing back in certain cases #19

Closed aearly closed 11 months ago

aearly commented 11 months ago

Prerequisites

Version (i.e. v2.x.x)

1.0.1-alpha.196

Node.js version (i.e. 18.x, or N/A)

N/A

Operating system

N/A

Operating system version (i.e. 20.04, 11.3, 10, or N/A)

N/A

Description

I've discovered a bug in Firefox where a NavigationEvent is not being triggered when navigating Back, and finally have a reproducible error case.

Steps to Reproduce

Fire up a web server with the examples directory, e.g. npx http-server -p 3000 ./example

Expected Behavior

Expected to see a navigation event for traversing backwards after a manual same-origin navigation plus a Navigation-API managed navigation.

This is very strange, I wonder why it only happens after a manual URL update.

fabiancook commented 11 months ago

Thanks for this, I’ll hopefully be able to get a fix together for it tomorrow!

fabiancook commented 11 months ago

I had tested this across Firefox Version 116.0.3 and Version 117.0 on macOS.

I thought I had this issue after updating... but then when I went to take the video it produced the extra log as expected.

https://github.com/virtualstate/navigation/assets/4337080/8eb6658f-7e80-4de3-b0ee-6a44aae86063

fabiancook commented 11 months ago

Oh! Running through the steps again, I do get the bug!

fabiancook commented 11 months ago

The popstate event has no state information on it, which is what we had been using to know for sure the original history state came from the navigation API or not. Seems like this should be changed up! Diving into it further now.

fabiancook commented 11 months ago

Bug:

https://github.com/virtualstate/navigation/assets/4337080/11e5362c-d486-419a-b517-9335cedb20f7

Fixed:

https://github.com/virtualstate/navigation/assets/4337080/02f55267-c8ba-49f8-bd9c-0b0752eb1a72

aearly commented 11 months ago

Thanks for fixing this! I wasn't able to test Safari -- did it have the same bug? I wondering if it's a spec quirk or a Firefox-only bug.

fabiancook commented 11 months ago

It would have been for any browser using this implementation for a polyfill. Might not have seen it in chrome/chromium because it's the native API and not the polyfill.

aearly commented 11 months ago

Verified fixed after integrating this latest version into my project. Thanks again!