fix 'about:blank' being displayed permanently on new tabs

[diracdeltas]

fix #8850

Test Plan:

1. open several new tabs. they should all show blank in the urlbar instead of about:blank after a short lag.
2. 'new tab button' and 'new tab signal' tests should pass.

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Ran git rebase -i to squash commits (if needed).

Test Plan:

Reviewer Checklist:

Tests

• Adequate test coverage exists to prevent regressions
• Tests should be independent and work correctly when run individually or as a suite ref
• New files have MPL2 license header

[bridiver]

this is a good workaround for now, but I want to capture the two underlying problems:

1. We update the urlbar location for the activeFrame only even though each frame has its own urlbar in the state. The urlbar should track location updates regardless of whether it is visible or not
2. We are updating the location too late on navigation. It should be updated when the provisional load succeeds, but we don't update it until the navigation completes

[bridiver]

both of these tests pass even when I remove the urlbar changes so I'm not sure if they really capture the issue

[diracdeltas]

they failed for me but maybe i was on a slightly older commit

[bridiver]

going to merge this as a workaround for now, but we want to get away from putting this kind of logic in components. The same workaround could be done in a reducer to update the urlbar location on tab change

[bbondy]

I don't understand what this does.

Only in the case thatthis.props.locationValue is about:blank does it go to UrlUtil.getDisplayLocation(this.props.location, pdfjsEnabled)) and UrlUtil.getDisplayLocation('about:blank', pdfjsEnabled)) actually returns about:blank.

[bbondy]

Since this doesn't do anything, I'm changing this to:

-        this.setValue(this.props.locationValue !== 'about:blank'
-          ? this.props.locationValue
-          : UrlUtil.getDisplayLocation(this.props.location, pdfjsEnabled))
+        if (this.props.locationValue !== 'about:blank') {
+          this.setValue(this.props.locationValue)
+        }

which I think is the expected functionality.

[bbondy]

I think in the timing I'm experiencing the change does nothing, but on the timing you're experiencing it does something. Trying to think of the right thing to do here.

[bbondy]

This is also the line of code that causes typing fast after new tab to get clobbered by the way. Very noticeable if you have lots of bookmarks.

[diracdeltas]

@bbondy the issue was that when i opened a new tab, this.props.locationValue is about:blank (and wouldn't change unless i typed in the urlbar) but this.props.location is about:newtab. So previously, when i opened a new tab, it would show about:blank forever. this seemed to only happen to some users, as noted in the original issue.

It seems like your change would cause the issue to re-appear.

[diracdeltas]

my understanding was that we should always be showing getDisplayLocation(this.props.location) unless the user is modifying the urlbar. That would be the case here if the user switched to a tab where they had been modifying the urlbar, but not when opening a new tab.