1732250 - "originalURI" from sessionHistory is identical to "url" when load request triggers a redirect

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

In our partial CDP implementation in Firefox we use the sessionHistory to retrieve the URL the user has originally typed into the locationbar. Now that I'm starting to make the test passing with Fission enabled, I can see that the originalURI is identical to the url property means the originally typed URL is lost:

https://treeherder.mozilla.org/logviewer?job_id=352339939&repo=try&lineNumber=26799

[task 2021-09-22T08:06:56.145Z] 08:06:56 INFO - TEST-UNEXPECTED-FAIL | remote/cdp/test/browser/page/browser_getNavigationHistory.js | History entry has the correct user typed URL set - Got "http://example.com/browser/remote/cdp/test/browser/page/doc_empty.html", expected "http://example.com/browser/remote/cdp/test/browser/page/sjs_redirect.sjs?http://example.com/browser/remote/cdp/test/browser/page/doc_empty.html"

For CDP it is implemented here:

https://searchfox.org/mozilla-central/rev/f62d42b1d98e67dc3da05d586f71103df02b8c4a/remote/cdp/domains/parent/Page.jsm#397

I assume that this is a regression from the session history in parent work. Note that it only happens when the original page redirects.

Comment 1

[Chris Peterson [:cpeterson]]

I assume that this is a regression from the session history in parent work.

In that case, I will send this bug to Fission triage to review whether this is a session history in parent ("SHIP") bug.

Comment 2

[Chris Peterson [:cpeterson]]

S3

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Is there some way to test this?

(I think https://searchfox.org/mozilla-central/rev/9e2239eb75e16204a6186c26f80aaa9723420919/docshell/base/CanonicalBrowsingContext.cpp#570 should special case original URI)

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Yes, as mentioned in comment 0 there is a browser chrome test, which permanently fails with Fission enabled. You can run it locally via the following mach command:

mach test remote/cdp/test/browser/page/browser_getNavigationHistory.js --setpref="fission.autostart=true" --setpref="remote.log.truncate=false"

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Looks like nsIChannel's originalURI doesn't work the way it is documented to work.
originalURI doesn't point to the right thing when AsyncOnChannelRedirect is called.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1732250, use the original URI from the old channel when updating session history entry, r=peterv

https://searchfox.org/mozilla-central/rev/1df999af9999ccb436512cfece57a68d94d36e08/netwerk/protocol/http/nsHttpChannel.cpp#5272
makes original uri handling in the channel rather magical. The value of it on the new channel is bogus during
AsyncOnChannelRedirect call, and nsIChannel.idl doesn't hint about that behavior.

browser_getNavigationHistory.js can work as a testcase once it is enabled for Fission.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2ff5136321a
use the original URI from the old channel when updating session history entry, r=peterv,necko-reviewers,valentin

Comment 8

[Cristina Cozmuta (:CrissCozmuta)]

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

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

The needinfo was provided

Comment 10

[Rob Wu [:robwu]]

For visibility in case people every stumble on this bug - many of the changes in the patch here were partially reverted in bug 1757458.