[Webkit-unassigned] [Bug 53897] [chromium] WebPageSerializerImpl doesn't serialize sub-frames correctly

[bugzilla-daemon at webkit.org]

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


Patrick R. Gansterer <paroga at paroga.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #81439|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #2 from Patrick R. Gansterer <paroga at paroga.com>  2011-02-06 18:32:00 PST ---
(From update of attachment 81439)
View in context: https://bugs.webkit.org/attachment.cgi?id=81439&action=review

Every patch needs a ChangeLog. See http://webkit.org/coding/contributing.html for mor information.

> Source/WebKit/chromium/src/WebPageSerializerImpl.cpp:330
>                          // Get the absolute link
> -                        String completeURL = param->document->completeURL(attrValue);
> +                        // handle iframe and frame tags

We usually write whole sentences. Should read: Handle iframe and frame tags.
Does this comment add any extra value? IMHO it does not, so we can remove it.
See webkit-dev thread for the actual discussion about comments: https://lists.webkit.org/pipermail/webkit-dev/2011-January/015767.html

> Source/WebKit/chromium/src/WebPageSerializerImpl.cpp:333
> +                        WebFrameImpl* subFrame = 
> +                            WebFrameImpl::fromFrameOwnerElement(
> +                                                const_cast<Element*>(element));

Is this const_cast really necessary?

> Source/WebKit/chromium/src/WebPageSerializerImpl.cpp:336
> +                        String completeURL = subFrame ? 
> +                            KURL(subFrame->url()) :
> +                            param->document->completeURL(attrValue);

Can you write this in one line only. Not need for 3 lines :-) Same for the subFrame above

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