1859299 - Add tests for navigation_started with redirects

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

[Julian Descottes [:jdescottes]]

As spotted in Bug 1859249, we don't have tests for redirects with navigation_started, we should add some to make sure we have the expected amount of events in those scenarios.

Comment 1

[Julian Descottes [:jdescottes]]

Hi Valentin,

While writing a test here for redirects, I noticed something unexpected when using the meta http-equiv=refresh in order to redirect to another page. Basically I have pageA.html which contains this meta with content="0;pageB.html". There's a first channel created to load pageA.html, and then a second one to load pageB.html. But this second channel still has pageA.html as originalURI. While I would expect this for a real redirect, I thought that for a refresh it would be set to pageB.html.

I am using this in a helper which attempts to monitor navigation events, and knowing the "target url" of the navigation can be helpful to match the beginning and the end of a navigation, as well as to ignore some duplicate notifications from WebProgressListener. According to the HTML spec, this refresh is a separate navigation from the first one, so its target url should be pageB.html.

Is this the expected behavior from nsIChannel here or if it's a bug? I can work around it rather easily, but I wanted to check with you first.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Great find, Julian. I think this is indeed a bug.
Most likely this is originating for the meta refresh code in docshell:
https://searchfox.org/mozilla-central/rev/0ba4632ee85679a1ccaf652df79c971fa7e9b9f7/docshell/base/nsDocShell.cpp#5212-5221

nsDocShell::ForceRefreshURI(nsIURI* aURI, nsIPrincipal* aPrincipal,
                            uint32_t aDelay) {
  NS_ENSURE_ARG(aURI);

  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aURI);
  loadState->SetOriginalURI(mCurrentURI);
  loadState->SetResultPrincipalURI(aURI);
  loadState->SetResultPrincipalURIIsSome(true);
  loadState->SetKeepResultPrincipalURIIfSet(true);
  loadState->SetIsMetaRefresh(true);

Comment 3

[Julian Descottes [:jdescottes]]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #2)

Great find, Julian. I think this is indeed a bug.
Most likely this is originating for the meta refresh code in docshell:
https://searchfox.org/mozilla-central/rev/0ba4632ee85679a1ccaf652df79c971fa7e9b9f7/docshell/base/nsDocShell.cpp#5212-5221

nsDocShell::ForceRefreshURI(nsIURI* aURI, nsIPrincipal* aPrincipal,
                            uint32_t aDelay) {
  NS_ENSURE_ARG(aURI);

  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aURI);
  loadState->SetOriginalURI(mCurrentURI);
  loadState->SetResultPrincipalURI(aURI);
  loadState->SetResultPrincipalURIIsSome(true);
  loadState->SetKeepResultPrincipalURIIfSet(true);
  loadState->SetIsMetaRefresh(true);

Good to know! Just in case, as I was trying to understand if this was intentional or not, I stumbled on this comment: https://searchfox.org/mozilla-central/rev/0ba4632ee85679a1ccaf652df79c971fa7e9b9f7/docshell/base/nsDocShellLoadState.h#443-447. Not sure it's completely relevant here, but just in case:

  // If a refresh is caused by http-equiv="refresh" we want to set
  // aResultPrincipalURI, but we do not want to overwrite the channel's
  // ResultPrincipalURI, if it has already been set on the channel by a protocol
  // handler.
  bool mKeepResultPrincipalURIIfSet;

Edit: It seems this was introduced for Bug 1468523

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1859299 - [wdspec] Add tests for browsingContext.navigationStarted with redirects

test_redirect_http_equiv is currently failing for Firefox, which might either be from a platform bug or something we have to workaround in
Bug 1859545

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adbe3798f7bc
[wdspec] Add tests for browsingContext.navigationStarted with redirects r=webdriver-reviewers,Sasha

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42666 for changes under testing/web-platform/tests

Comment 7

[Natalia Csoregi [:nataliaCs]]

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

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot