1842215 - Handle top-level pushState navigation

Status: RESOLVED FIXED

(Firefox Graveyard :: Shopping, defect, P2)

Description

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

It turns out some shopping websites, like walmart.com, actually use pushState to navigate within same-site links. The onLocationChange listener we're using in browser/base/content/browser.js filters out these navigations, so we aren't accurately detecting whether the user has navigated to/from product pages once they arrive on the site (unless they hard refresh the page).

We'll need to change how we detect navigation to ensure the product page check always fires on pushState-supported sites.

Comment 1

[:Gijs (he/him)]

This will be due to the LOCATION_CHANGE_SAME_DOCUMENT flag, fwiw. Just picking up onLocationChange without checking for that flag would work.

Comment 2

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

This will be due to the LOCATION_CHANGE_SAME_DOCUMENT flag, fwiw. Just picking up onLocationChange without checking for that flag would work.

Thanks, at Mossop's suggestion we're detecting navigations using TabsProgressListener.onLocationChange.

Is there a different spot we can insert into (not sure I see one), should I tack an update call into the spot where ReaderMode gets same-page navigation updates (https://searchfox.org/mozilla-central/source/browser/base/content/browser.js#5972), or should I create a webProgressListener just for the sidebar?

Comment 3

[:Gijs (he/him)]

(In reply to Jared Hirsch [:jhirsch] (he/him) (Needinfo please) from comment #2)

This will be due to the LOCATION_CHANGE_SAME_DOCUMENT flag, fwiw. Just picking up onLocationChange without checking for that flag would work.

Thanks, at Mossop's suggestion we're detecting navigations using TabsProgressListener.onLocationChange.

Is there a different spot we can insert into (not sure I see one), should I tack an update call into the spot where ReaderMode gets same-page navigation updates (https://searchfox.org/mozilla-central/source/browser/base/content/browser.js#5972), or should I create a webProgressListener just for the sidebar?

I think moving the call from this location to the reader mode block should work. Does that help or am I misunderstanding the question?

Comment 4

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Yup, that helps, thanks

Comment 5

[:Gijs (he/him)]

Stealing per discussion with Jared.

Comment 6

[:Gijs (he/him)]

Attachment: Bug 1842215 - handle pushState navigations between shopping pages, r?#shopping-reviewers

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df651cb43d5e
handle pushState navigations between shopping pages, r=shopping-reviewers,jhirsch

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/df651cb43d5e