[Webkit-unassigned] [Bug 55859] [Chromium] The method that retrieves all resources from a page should be moved from Chromium to WebKit

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #85884|review?                     |review-
               Flag|                            |




--- Comment #13 from Adam Barth <abarth at webkit.org>  2011-03-15 17:33:45 PST ---
(From update of attachment 85884)
View in context: https://bugs.webkit.org/attachment.cgi?id=85884&action=review

WebPageSerializer.cpp is such great sadness.  I re-wrote the whole thing, but never landed the new version.  :(

Almost every line of code in that file is wrong.

> Source/WebKit/chromium/src/WebPageSerializer.cpp:60
> +KURL getSubResourceURLFromElement(const Element& element)

I usually pass Elements by pointer, not by reference.

> Source/WebKit/chromium/src/WebPageSerializer.cpp:92
> +    if (value.isNull() || value.isEmpty() || value.startsWith("javascript:", false))

value.startsWith("javascript:", false) is wrong.  It could start with "   javascript:" also, for example.

> Source/WebKit/chromium/src/WebPageSerializer.cpp:106
> +    if ((element.hasTagName(HTMLNames::iframeTag) || element.hasTagName(HTMLNames::frameTag))
> +            && element.isFrameOwnerElement()) {

What if it's an object or an embed element that has a frame?

> Source/WebKit/chromium/src/WebPageSerializer.cpp:114
> +    if (url.isEmpty() || url.isNull() || !url.isValid())

isEmpty includes isNull, right?

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