[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





--- Comment #4 from public at fuzzac.com  2011-02-06 19:31:31 PST ---
(In reply to comment #2)
> (From update of attachment 81439 [details])
> 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.

Fixed.

> 
> > 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

Fixed.

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

Removed the const_cast and made fromFrameOwnerElement take a const argument instead.

> 
> > 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

Fixed.

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