Manual modification of URL hash in address/location bar

[mircea3]

If one begins modifying the URL manually, the address bar enters a temporary editing state, which exits when the user either navigates to that address (presses Enter) or cancels his changes (presses Escape).

If one does neither, the address bar remains in the editing state. The page continues to operate normally, but if code is changing `location.hash`, these changes are visually hidden, resulting in possible confusion.

This may seem acceptable, if the user himself left the address bar in that state. The more serious problem, however, is if the user is modifying only the hash part of the URL, then commits his change by pressing Enter. `location.hash` indeed gets updated, but the address bar does not exit the edting state, resulting in the confusing behaviour mentioned above. The only way to fully complete the operation seems to be to press Escape, as well.

It would be nice if pressing Enter in the address bar always exits the editing state.

Browser: palemoon-32.5.2.linux-x86_64-gtk3

[BenFenner]

Pressing [Enter] exits this "edit state" and visits the URL on my copy of Pale Moon.
Can you please give detailed steps to reproduce?

[mircea3]

On my computer it behaves this way:

1. Go to http://www.palemoon.org/ [Enter]
2. Edit the address bar: http://www.palemoon.org/#foo [Enter]
The triangle icon is still there ("Go to the address...")
3. In the console:
>> location.hash = '#bar'
The address bar is unchanged.
4. Click on the address bar. Press [Escape]
Triangle icon goes away and the address bar updates: http://www.palemoon.org/#bar

[BenFenner]

There may be multiple issues here, but the first issue seems to be that manually altering the anchor of a URL in the location bar, and then trying to get the browser to honor that new anchor by pressing Enter or clicking the green "Go to the address..." arrow does not cause the browser to reinterpret the anchor, nor revisit the URL with that anchor in mind.

Oddly enough, doing the same thing with a URL that does not contain an anchor (like http://www.palemoon.org/ ) does revisit the URL (very similar to an F5 refresh). I think some discussion is warranted about how the browser should behave when an anchor exists in the URL and the user asks to visit the same URL again either with the same anchor or a different one.

I'd think that if the anchor is changed and the URL is requested, at the very least the browser should attempt to reach that anchor point in the HTML document (without hitting the network connection). But even that doesn't happen, which is a little odd. Can't say I've ever run into this situation in the wild, but there it is.


Edit: Well testing on my own web forum software I was able to reproduce the issue, but wasn't able to on this forum's setup.

Code: Select all

// For example, first visit:
https://forum.palemoon.org/viewtopic.php?f=66&t=30275&start=40#p243198
// Then manually alter the URL to this and try to get it to apply:
https://forum.palemoon.org/viewtopic.php?f=66&t=30275&start=40#p243245

That actually works well, and as expected. Let me get an example that doesn't work, hang tight.

Edit 2: Never mind, it is behaving well on the other forum software now. I'll try to get an example that shows the poor behavior. We'll see.

Edit 3: Yah it must have been a total fluke when I checked before, or I was doing it wrong. This just seems to work. Here's an even better example to toggle between and see it working:

Code: Select all

https://forum.palemoon.org/#top
https://forum.palemoon.org/#start_here

I think the trouble comes in when the requested anchor ID does not exist on the page! That's when things stop working as expected. I think that's the thread to pull on now. For example, try:

Code: Select all

https://forum.palemoon.org/#start_here
https://forum.palemoon.org/#this_anchor_does_not_exist

[mircea3]

Yes, everything seems to work normally, except that it doesn't leave the editing state:

Using your example, try this:

1. https://forum.palemoon.org/ [Enter]
2. Scroll down
3. Add #top
4. Click on the address bar again
5. Press [Escape] and notice that it makes a difference! and it shouldn't

Only after pressing [Escape] is the address bar in the correct state, do you agree?

[Moonchild]

I think some discussion is warranted about how the browser should behave when an anchor exists in the URL and the user asks to visit the same URL again either with the same anchor or a different one.

This is as expected though. an anchor is a reference within a document. It's not part of the actual address and changing only the anchor does not trigger navigation. You don't want to trigger a reload when you're navigating within a document.

Only after pressing [Escape] is the address bar in the correct state, do you agree?

It does seem like that isn't as expected. Effectively, the address bar should return to its post-nav state when pressing enter even if the only thing changed is an anchor reference (i.e. display-mode) and it doesn't do that.

[Kris_88]

It does seem like that isn't as expected. Effectively, the address bar should return to its post-nav state when pressing enter even if the only thing changed is an anchor reference (i.e. display-mode) and it doesn't do that.

PM 32.5.2 tabbrowser.xml:

Code: Select all

                let isSameDocument =
                  !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
                // We need to clear the typed value
                // if the document failed to load, to make sure the urlbar reflects the
                // failed URI (particularly for SSL errors). However, don't clear the value
                // if the error page's URI is about:blank, because that causes complete
                // loss of urlbar contents for invalid URI errors (see bug 867957).
                if (this.mBrowser.didStartLoadSinceLastUserTyping() ||
                    ((aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) &&
                     aLocation.spec != "about:blank"))
                  this.mBrowser.userTypedValue = null;

FF 60.0 tabbrowser.js

Code: Select all

      let isSameDocument = !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
      // We need to clear the typed value
      // if the document failed to load, to make sure the urlbar reflects the
      // failed URI (particularly for SSL errors). However, don't clear the value
      // if the error page's URI is about:blank, because that causes complete
      // loss of urlbar contents for invalid URI errors (see bug 867957).
      // Another reason to clear the userTypedValue is if this was an anchor
      // navigation initiated by the user.
      if (this.mBrowser.didStartLoadSinceLastUserTyping() ||
          ((aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) &&
            aLocation.spec != "about:blank") ||
          (isSameDocument && this.mBrowser.inLoadURI)) {
        this.mBrowser.userTypedValue = null;
      }

I think it can be done this way:

tabbrowser.xml

Code: Select all

                let isSameDocument =
                  !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
                  
                // We need to clear the typed value
                // if the document failed to load, to make sure the urlbar reflects the
                // failed URI (particularly for SSL errors). However, don't clear the value
                // if the error page's URI is about:blank, because that causes complete
                // loss of urlbar contents for invalid URI errors (see bug 867957).
                // Another reason to clear the userTypedValue is if this was an anchor
                // navigation initiated by the user.

                if (this.mBrowser.didStartLoadSinceLastUserTyping() ||
                     isSameDocument ||
                    ((aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) &&
                     aLocation.spec != "about:blank"))
                  this.mBrowser.userTypedValue = null;

[Moonchild]

Seems like a good one-liner fix. I was thinking it would be something like that -- the primary issue would be to find out where to make the change (so thanks a lot for doing the legwork on that, Kris!). We just need to check for potential regressions.

[Kris_88]

We just need to check for potential regressions.

Certainly.

I must say that this part of the code is somehow too confusing, in my opinion... The actual mode switching is performed in the URLBarSetURI() function, and the modification of the urlbar in the SetPageProxyState() function (to put it simply). Both functions are in the browser.js file. And yes, I have not attempted to fully understand and test this mechanism, nor have I attempted to fully compare it to subsequent versions of FF.

[Moonchild]

The address bar is a very complex thing. It does autofill, history lookups, fragments, searches, various indicators, identity panel, etc. etc. so it's got several things split across various functions, events and triggers. and don't forget the urlbarbindings as well. So don't expect to just read it and understand all the intricacies in an afternoon