[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 #12 from public at fuzzac.com  2011-02-09 20:04:33 PST ---
(In reply to comment #10)
> (From update of attachment 81561 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=81561&action=review
> 
> >>> Source/WebKit/chromium/src/WebFrameImpl.cpp:1963
> >>> -WebFrameImpl* WebFrameImpl::fromFrameOwnerElement(Element* element)
> >>> +WebFrameImpl* WebFrameImpl::fromFrameOwnerElement(const Element* element)
> >> 
> >> Why is this change related to the problem you're trying to solve?  We almost never have "const Element*".
> > 
> > openTagToString is passed a const Element*. So I either had to remove the constness, cast it away or make fromFrameOwnerElement take a const argument. 
> > I thought the latter was the cleaner solution.
> 
> It's better to remove the constness.  "const Element" doesn't make any sense.

Removed the constness from the function argument.

> 
> >>> Source/WebKit/chromium/src/WebPageSerializerImpl.cpp:331
> >>> -                        String completeURL = param->document->completeURL(attrValue);
> >>> +                        WebFrameImpl* subFrame = WebFrameImpl::fromFrameOwnerElement(element);
> >>> +                        String completeURL = subFrame ? KURL(subFrame->url()) : param->document->completeURL(attrValue);
> >> 
> >> Why do we need to call KURL if we're trying to get a String in the end?
> > 
> > That's how it was done previously in that file, e.g in the constructor and completeURL(). I just stuck to the same style.
> 
> That probably wrong.  We should probably fix it.

Changed it to grab the url directly rather than calling KURL.

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