html icon indicating copy to clipboard operation
html copied to clipboard

Should location-object-navigate (and friends) queue a task to navigate instead of navigating directly?

Open zetafunction opened this issue 6 years ago • 10 comments

location-object-navigate doesn't currently specify that a task is queued to perform the navigation. However, it looks like some browsers do queue this:

document.body.appendChild(document.createElement('iframe'));window[window.length-1].location = 'javascript:console.log(2);';console.log(1);

In IE, this prints 2 and then 1. In Chrome, Firefox, and Safari, this prints 1 and then 2.

Similarly, should window.open() queue a task to navigate as well?

var w = window.open('javascript:window.opener.console.log(2)');console.log(1);

In Chrome and Safari, this prints 2 and then 1. In Firefox and IE, this prints 1 and then 2.

And there's also HTMLIFrameElement.src:

document.body.appendChild(document.createElement('iframe')).src = 'javascript:console.log(2);';console.log(1);

In Chrome, IE, and Safari, this prints 2 and then 1. In Firefox, this prints 1 and then 2.

Queueing a task would be consistent with https://html.spec.whatwg.org/multipage/links.html#following-hyperlinks-2, but it's not clear to me it's the right thing to do.

I'm not sure of a way to test this hypothesis without using javascript: URLs: it's entirely possible that the reason Firefox always prints 1 before 2 is because it is the only implementation that queues a task to process javascript: URLs.

zetafunction avatar Jun 01 '18 21:06 zetafunction

How would it throw an exception if it queued a task?

annevk avatar Jun 02 '18 05:06 annevk

Doesn't the issue of throwing exceptions also apply to hyperlink navigations? So following that line of reasoning, then maybe nothing should be queued?

zetafunction avatar Jun 02 '18 07:06 zetafunction

No, that doesn't set the exceptions enabled flag.

annevk avatar Jun 02 '18 13:06 annevk

Is the below test correct, if we assume that no task should be queued(cc https://github.com/servo/servo/pull/23368)?

https://github.com/web-platform-tests/wpt/blob/bb11ac51f133591d7759c991ed822b02db05f897/html/semantics/scripting-1/the-script-element/execution-timing/028.html

This test uses location, and expects the second log to happen before the JS url execution:

log('inline script #1');
window.location.replace('javascript:log(\'JS URL\')');
log('end script #1');

Wouldn't this only happen if a task was queued? (Or is the expected delay related to execution of the newly created script?)

gterzian avatar May 14 '19 02:05 gterzian

Also, just to point it out, in the case of the anchor element, following-hyperlinks says to "queue a task to navigate".

Then, inside navigate, we are told to queue a task to execute a JS url request.

So this means that in the examples discussed above, while no task should be queued to navigate(which includes unloading and so on), a task should be queued to perform step 13, in the case that it involves a JS url.

So the test I mentioned above,/html/semantics/scripting-1/the-script-element/execution-timing/028.html is probably "correct" in that it doesn't rely on the navigation itself queuing as task, I think, just step 13 of it.

On the other hand, related the anchor element, there is also /html/semantics/scripting-1/the-script-element/execution-timing/029.html. That test somehow seems to expect that a task is queued for the navigation, which matches the spec(since anchor element queue tasks to follow their hyperlinks), however the test seems to also expect that the JS url is processed as part of that same "navigate task", not as a subsequent task queued as part of navigation. Or at least that is what I encountered when working on this at https://github.com/servo/servo/pull/23368. I tried following the spec "to the letter" and actually doing the navigation in a task, with the JS url done in a task queued by the navigation task, but that resulted in the JS url executing only after the load event fired(so "JS URL" was not even in the list of events logged in the test).

So the test suite seems to expect that when navigate is done in a task(in the case of an anchor following a hyperlink), no task is enqueued for the JS url part, and when navigate is not done in a task, a task is enqueued for the JS url part.

I haven't looked into window.open yet, but I assume it's a similar situation. Perhaps no task is queued for the navigation, but a task is queued by some engines to execute the JS url.

Since executing the JS url involves manipulating the realm of JS objects(I think), it makes sense to do thart part in a task on the event-loop. Note that otherwise step 13 is done "in-parallel".

gterzian avatar May 14 '19 03:05 gterzian

Update: In the context of window.open, if you don't queue a task for thh JS url exec, you get an error with this test because this.checkValues is not a function inside the JS url passed to open.

That's because the tests only sets checkValues on the return value of open after the call to open. So the test seems to strongly expect that the JS url is only executed in a subsequent task, so that the current task gets a change to set checkValues.

gterzian avatar May 14 '19 03:05 gterzian

So all in all, I'd go for the following:

  1. It's right to queue a task to execute a JS url in all cases, except maybe following hyperlinks.
  2. It's weird to queue a task to execute a JS url in following hyperlinks, since that means the JS url is only executed after at least two "turns of the loop", since the initial navigation steps are already done in a task. /html/semantics/scripting-1/the-script-element/execution-timing/029.html seems to expect that only one task is queued to do the navigation, and execute the JS url in that same task.
  3. If you try never enqueuing a task to execute the JS url(but still do the navigation of following a hyperlink inside a task), /execution-timing/029.html passes, however a few other test fail, including on unrelated test that is using window.open. So since more tests fail, and even seemingly unrelated ones, I'd say that this probably isn't the right approach.

So at this point the spec could perhaps say "unless this navigation is the result of running the "following-hyperlinks" algorithm, in which case the following steps should be done in the same task that invoked the navigation steps? Would need a bit of reshuffle at step 12 too, so that it doesn't read "continue running these steps in parallel" in that particular case...

gterzian avatar May 14 '19 04:05 gterzian

there is also /html/semantics/scripting-1/the-script-element/execution-timing/029.html

This test is, as you note, not following the spec. Note that it's marked as failing in Firefox... It also fails in Safari, per https://wpt.fyi/results/html/semantics/scripting-1/the-script-element/execution-timing/029.html?label=master&product=chrome%5Bexperimental%5D&product=edge&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D&aligned&q=html%2Fsemantics%2Fscripting-1%2Fthe-script-element%2Fexecution-timing%2F029.html

It seems to be passing in Chrome for reasons that are entirely unclear to me, but Chrome is making a bunch of changes in this area right now to align with the spec, so I suspect the right thing is to just fix this test to follow the spec.

That said, last I checked there were suggestions that the spec (and implementations) be changed to not queue an extra task when following hyperlinks. I do NOT thing we should add special-casing in navigation for the hyperlink case.

bzbarsky avatar May 14 '19 04:05 bzbarsky

Ok, thanks for the info, I will happily mark /execution-timing/029.html as failing in Servo too, and proceed with following the current spec to the letter..

Actually I'll look at changing the test instead...

gterzian avatar May 14 '19 04:05 gterzian

What @bzbarsky said. I think navigation (and form submission too as a special case) should be invoked synchronously and not involve additional tasks that can result in changing state and such.

annevk avatar May 14 '19 11:05 annevk

Update: in https://github.com/whatwg/html/pull/6315 we're speccing navigation to generally start synchronously. That is, neither location.href nor following hyperlinks will inherently queue a task.

However, javascript: URL navigation is always done in a task. So there will always be a single task for javascript: URL navigation.

Tests are added for that in https://github.com/web-platform-tests/wpt/pull/36358, and at least Firefox and Chrome pass them.

I will also look into fixing the 029.html test.

domenic avatar Oct 10 '22 08:10 domenic