548263 - XML content sink doesn't split rel value properly before looking for dns-prefetch

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[Henri Sivonen (:hsivonen)]

The HTML content sink has this code:
2793       // look for <link rel="next" href="url">
2794       nsAutoString relVal;
2795       element->GetAttr(kNameSpaceID_None, nsGkAtoms::rel, relVal);
2796       if (!relVal.IsEmpty()) {
2797         // XXX seems overkill to generate this string array
2798         nsAutoTArray<nsString, 4> linkTypes;
2799         nsStyleLinkElement::ParseLinkTypes(relVal, linkTypes);
2800         PRBool hasPrefetch = linkTypes.Contains(NS_LITERAL_STRING("prefetch"));
2801         if (hasPrefetch || linkTypes.Contains(NS_LITERAL_STRING("next"))) {
2802           nsAutoString hrefVal;
2803           element->GetAttr(kNameSpaceID_None, nsGkAtoms::href, hrefVal);
2804           if (!hrefVal.IsEmpty()) {
2805             PrefetchHref(hrefVal, element, hasPrefetch);
2806           }
2807         }
2808         if (linkTypes.Contains(NS_LITERAL_STRING("dns-prefetch"))) {
2809           nsAutoString hrefVal;
2810           element->GetAttr(kNameSpaceID_None, nsGkAtoms::href, hrefVal);
2811           if (!hrefVal.IsEmpty()) {
2812             PrefetchDNS(hrefVal);
2813           }
2814         }
2815       }

But the XML content sink only has this code:
661     // Look for <link rel="dns-prefetch" href="hostname">
662     if (nodeInfo->Equals(nsGkAtoms::link, kNameSpaceID_XHTML)) {
663       nsAutoString relVal;
664       aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::rel, relVal);
665       if (relVal.EqualsLiteral("dns-prefetch")) {
666         nsAutoString hrefVal;
667         aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::href, hrefVal);
668         if (!hrefVal.IsEmpty()) {
669           PrefetchDNS(hrefVal);
670         }
671       }
672     }

The XML content sink should have the same code that the HTML content sink has here.

Comment 1

[daniloshiga]

Attachment: Copied the code that handles rel value from HTML content sink.

If I understood the bug correctly, this should fix it.

Comment 2

[Henri Sivonen (:hsivonen)]

Comment on attachment 547683 [details] [diff] [review]
Copied the code that handles rel value from HTML content sink.

Looks ok.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

the patch doesn't apply anymore, could you please update it and then reask for c-n? be sure to put checkin-needed in the keywords field

Comment 4

[daniloshiga]

Attachment: new version of the patch, since older one doesn't apply anymore

just made the same changes, based on the current code.

Comment 5

[Ed Morley [:emorley]]

What name do you want for the patch author? I've set it to just daniloshiga for now, but if you let me know before this gets pushed to inbound, I'll adjust for you.

Sent to try:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=9a567d609584

Assuming all green, will push to inbound after.

I'm happy to fix it this time, but for new patches could you make sure the patch format (context, commit message, author etc) is correct when requesting checkin-needed - thanks! :-)
http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Comment 6

[Ed Morley [:emorley]]

Failed try:
http://tbpl.allizom.org/php/getParsedLog.php?id=6056159#error0
{
/builds/slave/try-lnx64-dbg/build/content/xml/document/src/nsXMLContentSink.cpp: In member function 'virtual nsresult nsXMLContentSink::CloseElement(nsIContent*)':
/builds/slave/try-lnx64-dbg/build/content/xml/document/src/nsXMLContentSink.cpp:652:10: error: 'relVal' was not declared in this scope
make[8]: *** [nsXMLContentSink.o] Error 1
}

Comment 7

[daniloshiga]

Attachment: new version of patch, this time done right.

I'm really sorry about the last patch, done it in a hurry and used the wrong command, lesson learned, here comes the right patch.

Comment 8

[Ed Morley [:emorley]]

That's ok - thanks for fixing it up :-)

The patch is now in my queue with a few other bits that are being sent to try first and then landing on mozilla-inbound. Once there it will be merged to mozilla-central within a day or so. At each stage the relevant changeset URLs will be posted back here and whomever does the merges will mark resolved fixed at the appropriate point.

Thanks again for your contribution - looking forward to seeing the next one - ask on IRC if you'd like any guidance/suggestions! :-)

Comment 9

[Ed Morley [:emorley]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7658f61d9e2b

Comment 10

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/7658f61d9e2b