[webkit-reviews] review canceled: [Bug 85118] REGRESSION(r95249): Iframes are printed blank : [Attachment 139879] Iframe printing fix.

[bugzilla-daemon at webkit.org]

Vitaly Buka <vitalybuka at chromium.org> has canceled  review:
Bug 85118: REGRESSION(r95249): Iframes are printed blank
https://bugs.webkit.org/show_bug.cgi?id=85118

Attachment 139879: Iframe printing fix.
https://bugs.webkit.org/attachment.cgi?id=139879&action=review

------- Additional Comments from Vitaly Buka <vitalybuka at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=139879&action=review


>>>> Source/WebCore/page/Frame.cpp:545
>>>> +		  return resultSize;
>>> 
>>> This does not seem like the right approach. We should have some cleaner way
to avoid trying to work with originalSize rather than relying on the fact that
its width is zero. I think the change should be at the printing-specific call
site rather than here in this function.
>> 
>> (In reply to comment #30)
> 
> 

Done.

>> Source/WebCore/rendering/RenderView.cpp:652
>> +bool RenderView::usePrintingLayout() const
> 
> This function sounds like something that tells the view to use a printing
layout. That’s why we use names like shouldUsePrintingLayout.

Done.

>> Source/WebCore/rendering/RenderView.cpp:658
>> +	return tree && !tree->parent();
> 
> Since tree can’t be null here is how this should be written:
> 
>     if (!printing() || !m_frameView)
>	  return false;
>     Frame* frame = m_frameView->frame();
>     return frame && !frame->tree()->parent();

Done.