Closed Bug 1134648 Opened 5 years ago Closed 4 years ago
dns-prefetch is not happening when link is appended to the document after window
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.111 Safari/537.36 Steps to reproduce: This test: http://jsbin.com/domuniqihe/2/edit Appends a <link rel="dns-prefetch" href="…"> to document.head after window.onload Actual results: The DNS was not prefetched, and the DNS lookup happens when the image is appended to the document after a 3 second timeout. A waterfall can be seen here: http://www.webpagetest.org/result/150219_K3_6f4b266d4799d6afa98c95e8acbdd009/1/details/ Expected results: The browser should pick up the added link tag to the document and prefetch the host like it does for already present link element while parsing the document, 3 seconds before the image loads from http://fortunes.inertie.org/custom/img/h1.png A waterfall like the one Chrome displays: http://www.webpagetest.org/result/150219_86_23f61a43ed4f7d9ec5d99fe88472e50e/1/details
Component: Untriaged → Networking: DNS
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can take a look.
Looking at this, it seams that PrefetchDNS is never called. I will ask over there.
Assignee: dd.mozilla → nobody
PrefetchDNS is called by the parser. So it clearly won't be called for links that are added via a non-parser mechanism.
Ah, I had assumed that this code would take care of it: https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLAnchorElement.cpp?from=HTMLAnchorElement.cpp#157 but apparently not. Hopefully someone from the DOM side of things can look into the plumbing we're missing here. Andrew, do you know who could take this?
Flags: needinfo?(jduell.mcbugs) → needinfo?(overholt)
(In reply to Jason Duell [:jduell] (needinfo? me) from comment #4) > Ah, I had assumed that this code would take care of it: > > > https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLAnchorElement. > cpp?from=HTMLAnchorElement.cpp#157 > > but apparently not. Hopefully someone from the DOM side of things can look > into the plumbing we're missing here. Andrew, do you know who could take > this? Well sure, that would take care of it for a <a> tag. But this is a <link>, which has no corresponding code in BindToTree, AFAICT.
I have adde similar thing that HTMPAnchorElement does. I am not really familiar with this code.
Attachment #8567019 - Flags: feedback?(khuey)
This sounds similar to bug 580313, Do we actually have a reasonable spec for this stuff? (I doubt) But this should be fixed anyhow, and relevant spec bugs filed.
@Olli: Yes, it's the exact same issue in disguise.. FF is not processing dynamically inserted <link> relations -- applies to prefetch, dns-prefetch, etc. Re, spec: There is a mention for this in , but I'm sure it can be fleshed out further and be improved. If you have any suggestions, let me know.  https://w3c.github.io/preload/#dynamic-scheduling
Olli: opened a bug to track this: https://github.com/w3c/resource-hints/issues/25. If you have any suggestions for appropriate spec language + behaviors, that'd be much appreciated :)
Comment on attachment 8567019 [details] [diff] [review] bug_1134648_v1.patch I don't think I'm the right person to review this. Maybe smaug?
Attachment #8567019 - Flags: feedback?(khuey) → feedback?(bugs)
Olli: I wrote an additional test to look at why Chrome is doing: http://jsbin.com/kuluqipuba/2 http://www.webpagetest.org/result/150223_MP_302a68179e1c39b71532694b3e415a75/1/details/ It looks like the DNS isn't prefetched when the LINK element isn't appended to the Document. I'm wondering if Firefox should actually introduce this, but perhaps it would be more productive to move the specs to another discussion, and aim for feature equality with an existing browser in this bug?
Comment on attachment 8567019 [details] [diff] [review] bug_1134648_v1.patch Why do we need the flags? We have that information already as attribute value.
Attachment #8567019 - Flags: feedback?(bugs) → feedback-
*bump* Anything we can do to unblock this and related dynamic-hints use cases?
Apparently <a> uses similar flags after all, which is a bit odd, but decided to reused that setup, even if it has some unexpected behavior like PREFETCH_REQUESTED isn't necessarily ever unset etc. But need to take care of changes to attributes too and actually check there is rel="dns-prefetch" etc. But I don't know how to test this automatically. Do we have any way for that? Bug 622232 didn't add tests. sworkman do you know? (I used wireshark locally, and need to still go through all the different cases.) Filed spec bugs https://github.com/w3c/resource-hints/issues/48 and https://github.com/w3c/resource-hints/issues/49 (looks like we require prefetch element to be in a document which has browsing context, but document doesn't need to be the active one. See nsHTMLDNSPrefetch::IsAllowed)
Comment on attachment 8692561 [details] [diff] [review] v3 https://treeherder.mozilla.org/#/jobs?repo=try&revision=9829fe31bbfa parser/html/nsHtml5DocumentBuilder.cpp/dom/xml/nsXMLContentSink.cpp shouldn't need to deal with dns-prefetch. In fact, because of them we trigger dns-prefetch even for elements which aren't in document when innerHTML = "<link ...>" is used on an element which isn't in document. But still don't know whether we have any way to test this automatically.
Comment on attachment 8692561 [details] [diff] [review] v3 Review of attachment 8692561 [details] [diff] [review]: ----------------------------------------------------------------- feedback+ Looks somewhat sane to me, but you should def trust bz more than me. It's been a while since I worked on the DNS prefetch code, and I wasn't super familiar with the calling code at the time, never mind now ;) (In reply to Olli Pettay [:smaug] from comment #15) > But I don't know how to test this automatically. Do we have any way for that? > Bug 622232 didn't add tests. sworkman do you know? > (I used wireshark locally, and need to still go through all the different > cases.) No, we didn't have a way to do automated testing for this back then, and I don't think we have anything now either. But Dragana or Jduell will have more up-to-date knowledge.
Attachment #8692561 - Flags: feedback?(sworkman) → feedback+
I used wireshark for testing, but anything automatic? Dragana, do you happen to know? Or perhaps you have some manual tests which you could run?
Comment on attachment 8692561 [details] [diff] [review] v3 r=me
Attachment #8692561 - Flags: review?(bzbarsky) → review+
Hooray! Curious, do we now also need to mirror this for prefetch as well? https://bugzilla.mozilla.org/show_bug.cgi?id=580313
(In reply to Olli Pettay [:smaug] from comment #19) > I used wireshark for testing, but anything automatic? Dragana, do you happen > to know? > Or perhaps you have some manual tests which you could run? I do not know any automatic tests for this. I think I was using logging to be test the first patch, but nothing automatic.
You need to log in before you can comment on or make changes to this bug.