[Webkit-unassigned] [Bug 56650] [Chromium] The WebPageSerializer::retrieveAllResources() should retrieve CSS resources

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=56650





--- Comment #8 from Adam Barth <abarth at webkit.org>  2011-03-18 18:22:34 PST ---
(From update of attachment 86223)
View in context: https://bugs.webkit.org/attachment.cgi?id=86223&action=review

>>> Source/WebKit/chromium/src/WebPageSerializer.cpp:143
>>> +            if (!url.isEmpty() && !url.isNull() && url.isValid()
>>> +                && (url.protocolIs("http") || url.protocolIs("https") || url.protocolIs("file"))) {
>>> +                resourceURLs->add(url);
>>> +            }
>> 
>> You should do the JavaScript URL check together with these.  Also "!url.isEmpty() && !url.isNull() && url.isValid()" are all redundant with the later checks, right?
> 
> Looking at KURL.cpp, it seems we still need isValid(), but you are right we don't need the rest or even to check for the JS protocol.

Ok.

>>> Source/WebKit/chromium/src/WebPageSerializer.cpp:161
>>> +    // If we have already seen that frame, ignore it.
>> 
>> This comment is a "what" not a "why" and should be removed.
> 
> I thought the comment useful as it is not really obvious that ListHashSet::add() returns a std::pair with second being a bool that indicates whether the element was inserted.

That's a pretty nutty API...  It's probably not worth fixing the API in this patch.  Feel free to leave the comment if think it's helpful.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.