`rel="icon"` requests do not fire since it doesn't translate to a valid preload destination

[TimvdLippe]

What is the issue with the HTML Standard?

Links with rel="icon" run the default fetch and process the linked resource. The first step is to create link options from element where the destination is the result of translating a preload destination. The second step then creates a link request.

When creating a link request, since "icon" is not a valid destination as preload destination, its destination is null. Therefore, it states "If options's destination is null, then return null.".

Step 3 of "default fetch and process the linked resource" then states "If request is null, then return." and nothing happens.

Step 5 of "default fetch and process the linked resource" does call into icon resource fetch steps which does set the destination to Image, but at that point it is too late since we already bailed out at step 3.

Servo ran into this when correctly returning null when computing the preload destination (servo/servo#39549).

Should step 2 of https://html.spec.whatwg.org/multipage/semantics.html#create-a-link-request instead be "If options's destination is null, then set options's destination to the empty string"? If I make that change in Servo, the previous WPT failures (servo/servo#39549 (comment)) pass once again.

[annevk]

As currently defined "link processing options"'s "destination" member can only hold a string, not null. So I think there are several issues here.

[zcorpan]

Reopening for @TimvdLippe's comments #11727 (review)

[noamr]

Reopening for @TimvdLippe's comments #11727 (review)

It's not a noop...
It is set for preloads: in https://html.spec.whatwg.org/#link-type-preload:link-options-destination-3 (<link rel=preload> element) and in https://html.spec.whatwg.org/#processing-link-headers:link-options-destination (
preload headers).

It's a noop for other link types but I guess that's expected because they have a fixed destination?

Suggesting to close this @TimvdLippe. The other typo review comment was fixed by an editorial PR.

[TimvdLippe]

The no-op is for prefetch specifically, it doesn't use the preload steps as I see it. Will read once more later this week.

Still, the other comment about nullable destination still applies in the linked comment. We had to remove that line to make it work in Servo.

[noamr]

The no-op is for prefetch specifically, it doesn't use the preload steps as I see it. Will read once more later this week.

It's by design. Prefetch doesn't support the as attribute.

Still, the other comment about nullable destination still applies in the linked comment. We had to remove that line to make it work in Servo

Is that for prefetch?

Nullable would be the case if translating the preload destination fails, and only for preload. In that case preloading would bail out early.

[noamr]

@TimvdLippe
Note that the default for destination is now the empty string and not null. Only preloading can set it to null. So sounds like you have not changed the default destination to the empty string, as per the spec PR?

[TimvdLippe]

servo/servo#39549 is the reason this issue was opened. One of the changes it made was changing LinkProcessingOptions::destination from Option<Destination> to Destination. In other words: before it was nullable, now it isn't.

It is always initialized to Destination::None which is the empty string in our case. The cases where destination is set to a different value are:

1. https://html.spec.whatwg.org/multipage/semantics.html#apply-link-options-from-parsed-header-attributes where step 1.3 checks for null, so destination is always non-null. Implemented in Servo here: https://github.com/servo/servo/blob/5887e1e963faf5eec9c25a0528b4d77a335eb5e3/components/script/dom/processingoptions.rs#L114-L120
2. https://html.spec.whatwg.org/multipage/links.html#link-type-preload%3Afetch-and-process-the-linked-resource-2 where step 4 and 5 do the exact same. Implemented in Servo here: https://github.com/servo/servo/blob/5887e1e963faf5eec9c25a0528b4d77a335eb5e3/components/script/dom/html/htmllinkelement.rs#L784-L791

Therefore, LinkProcessingOptions::destination is always non-null and we adhere to the spec. When we look at usages of LinkProcessingOptions::destination, we see https://html.spec.whatwg.org/multipage/semantics.html#fetching-and-processing-a-resource-from-a-link-element:link-options-destination Given that it is always non-null (it's the default empty string or it is assigned to an existing value), this check is redundant. It can never be null, so it can never return null in this algorithm. In servo/servo#39549 we there had to remove this line to make it work: see servo/servo#39549 (comment)

Then for the assignment of the empty string to options.destination in the prefetch link algorithm. The spec is https://html.spec.whatwg.org/multipage/links.html#link-type-prefetch:fetch-and-process-the-linked-resource-2 which Servo implements here: https://github.com/servo/servo/blob/5887e1e963faf5eec9c25a0528b4d77a335eb5e3/components/script/dom/html/htmllinkelement.rs#L509-L520 We construct a new LinkProcessingOptions per https://html.spec.whatwg.org/multipage/semantics.html#create-link-options-from-element Since step 2 does not set a destination, we initialize it to the empty string: https://github.com/servo/servo/blob/5887e1e963faf5eec9c25a0528b4d77a335eb5e3/components/script/dom/html/htmllinkelement.rs#L429 Immediately afterwards in step 3 of prefetch fetching (https://html.spec.whatwg.org/multipage/links.html#link-type-prefetch:link-options-destination) we once again set it to the empty string. Since the object already has that value, I believe that step 3 here is redundant, since it was already set to it when constructing options.

Opened #11757 with the changes as I see it.