Bug 1766145 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

### STR
1. First try going to [this link](https://github.com/aminomancer/uc.css.js)
2. Click the urlbar and hit Enter
3. Doc reloads
4. Now go to [this link](https://github.com/aminomancer/uc.css.js#updating)
5. Click the urlbar and hit Enter
6. Nothing happens, because same-document navigation only scrolls to the named anchor "updating"

This is obviously intended behavior in a manner of speaking, because the implementation is really low level and just cares about the load states. It's just checking if the destination URI equalsExceptRef the current URI. This is what we want generally, so that we can click named anchor links to jump to the headings, as in the table of contents I linked.

But with respect to the urlbar, I think we want something a bit different. When we click a named anchor link it's unlikely that it will do nothing, because the named anchor link is unlikely to be at the same scroll position as the heading to which it links. But in the urlbar it's trivially easy for us to trigger a load that does nothing: we just have to click the urlbar and hit Enter.

If we typed a different fragment after the hash then again, it would make sense to use the same-document navigation handler. But if the urlbar value is identical to the current doc spec, and the scroll position hasn't changed, the same-document navigation handler will do nothing. So this can feel like a bug to some users ([see here for example](https://github.com/aminomancer/uc.css.js/issues/39)).

I have two ideas for resolving this. The first and simpler, I already implemented to make a custom script for that person who first reported this. Basically, in `gURLBar._loadURL` we check if `url === browser.currentURI.spec` and `where === "current"` and if so, we go into a different path where our ultimate `browser.loadURI` call will pass the `LOAD_FLAGS_IS_REFRESH` flag. So that means pressing Enter in the urlbar when its value equals the current URL will always cause the navigation to reload.

However, that isn't a perfect solution. What happens if we go to [the second link](https://github.com/aminomancer/uc.css.js#updating) and then scroll down a bunch, and then hit Enter so we can get back up to the original heading? Well Firefox currently handles this just as any other named anchor navigation. But with the first proposed change, this would trigger an unnecessary reload. How do we know whether the user wants a full reload, or just anchor navigation?

I would propose that we move this change from the javascript to the docshell code, and in there we add a special case within same-document navigation that yields to a normal URI load:
1. It's not history navigation
2. URIs are the same _including_ hash
3. Needed scroll position is the same as current scroll position

I'm not sure exactly how to implement #3 but I can see that the resources exist for us to compare the two from within `nsDocShell::InternalLoad`. Anyway, it seems like that would be the ideal solution. But I already have a patch for the easier javascript approach, so I'm gonna submit it as WIP in case anyone wants to see how that works in practice.
### STR
1. First try going to [this link](https://github.com/aminomancer/uc.css.js)
2. Click the urlbar and hit Enter
3. Doc reloads
4. Now go to [this link](https://github.com/aminomancer/uc.css.js#updating)
5. Click the urlbar and hit Enter
6. Nothing happens, because same-document navigation only scrolls to the named anchor "updating"

This is obviously intended behavior in a manner of speaking, because the implementation is really low level and just cares about the load states. It's just checking if the destination URI equalsExceptRef the current URI. This is what we want generally, so that we can click named anchor links to jump to the headings, as in the table of contents I linked.

But with respect to the urlbar, I think we want something a bit different. When we click a named anchor link it's unlikely that it will do nothing, because the named anchor link is unlikely to be at the same scroll position as the heading to which it links. But in the urlbar it's trivially easy for us to trigger a load that does nothing: we just have to click the urlbar and hit Enter.

If we typed a different fragment after the hash then again, it would make sense to use the same-document navigation handler. But if the urlbar value is identical to the current doc spec, and the scroll position hasn't changed, the same-document navigation handler will do nothing. So this can feel like a bug to some users ([see here for example](https://github.com/aminomancer/uc.css.js/issues/39)).

I have two ideas for resolving this. The first and simpler, I already implemented to make a custom script for that person who first reported this. Basically, in `gURLBar._loadURL` we check if `url === browser.currentURI.spec` and `where === "current"` and if so, we go into a different path where our ultimate `browser.loadURI` call will pass the `LOAD_FLAGS_IS_REFRESH` flag. So that means pressing Enter in the urlbar when its value equals the current URL will always cause the navigation to reload.

However, that isn't a perfect solution. What happens if we go to [the second link](https://github.com/aminomancer/uc.css.js#updating) and then scroll down a bunch, and then hit Enter so we can get back up to the original heading? Well Firefox currently just does nothing. But with the first proposed change, this would trigger an unnecessary reload. How do we know whether the user wants a full reload, or just anchor navigation?

I would propose that we move this change from the javascript to the docshell code, and in there we add a special case within same-document navigation that yields to a normal URI load:
1. It's not history navigation
2. URIs are the same _including_ hash
3. Needed scroll position is the same as current scroll position

I'm not sure exactly how to implement #3 but I can see that the resources exist for us to compare the two from within `nsDocShell::InternalLoad`. Anyway, it seems like that would be the ideal solution. But I already have a patch for the easier javascript approach, so I'm gonna submit it as WIP in case anyone wants to see how that works in practice.

Back to Bug 1766145 Comment 0