Bug 88201 – REGRESSION (r118039): Incorrect formatting of pdf when printing from Reader view

• Component: Printing
• Status: RESOLVED FIXED

[Wade]

Created attachment 145496 [details]
screenshot of resulting incorrect formatting

regardless of site, when the Reader function is used and I try to print, the resulting pdf is formatted horribly incorrectly, making a two page document a 200 page document, for instance.

[Wade]

forgot to mention- 'official' version of Safari is not showing same error.

[Alexey Proskuryakov]

<rdar://problem/11569133>

[Brady Eidson]

This is about iframes, and broke in http://trac.webkit.org/changeset/118039

[Vitaly Buka]

Is it possible to export to html Reader result?
Or create layout test with broken feature?

[Wade]

additional point- from the Reader view, the formatting for the email function is just fine. It is only the PDF export that formats incorrectly.

[Wade]

additional point- from the Reader view, the formatting for the email function is just fine. It is only the PDF export that formats incorrectly.

[Brady Eidson]

(In reply to comment #5)
> additional point- from the Reader view, the formatting for the email function is just fine. It is only the PDF export that formats incorrectly.

For the sake of the WebKit engineers, "PDF export" is not specifically interesting.  "Printing mode" is what is interesting.

[Vitaly Buka]

Does any one work on this?
I can try to localize, but there is probability that Reader does something to compensate bug fixed in my patch.

[Alexey Proskuryakov]

> I can try to localize, but there is probability that Reader does something to compensate bug fixed in my patch.

I don't know if Reader does something unusual here, but it certainly doesn't try to compensate for bug 85118 - simply because Safari never shipped with a version of WebKit that had this problem.

[Vitaly Buka]

(In reply to comment #9)
> > I can try to localize, but there is probability that Reader does something to compensate bug fixed in my patch.
> 
> I don't know if Reader does something unusual here, but it certainly doesn't try to compensate for bug 85118 - simply because Safari never shipped with a version of WebKit that had this problem.

My CL fixed two independent issues:
1. Source/WebCore/page/Frame.cpp. It was not shipped with with Safari. And it's unrelated to this issue.
2. Source/WebCore/rendering/RenderView.cpp. It was shipped and present for a long time. For most sites it's just a little strange iframe content when printing. But http://code.google.com/p/chromium/source/search is not printed at all. 

If you revert just RenderView.cpp or just use official Safari you'll see difference on layout tests attached to that bug. Content of iframes alined incorrectly after printing.

[Vitaly Buka]

> If you revert just RenderView.cpp or just use official Safari you'll see difference on layout tests attached to that bug. Content of iframes alined incorrectly after printing.

Actually I am not so sure. I need to experiment with this a little more.

[Vitaly Buka]

I believe I understand the issue.
Base assumption of my CL was that only root frame should handle page size. It ok for regular printing. However Reader requests to print subframe only. So correct condition should be to check page size in closest to root printed frame.

I'll send patch for review soon.

[Vitaly Buka]

Created attachment 145716 [details]
Regression fix for Reader printing.

[WebKit Review Bot]

Attachment 145716 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/page/Frame.cpp:539:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/page/Frame.cpp:540:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Vitaly Buka]

Created attachment 145717 [details]
Regression fix for Reader printing.

[Brady Eidson]

Comment on attachment 145717 [details]
Regression fix for Reader printing.

View in context: https://bugs.webkit.org/attachment.cgi?id=145717&action=review

> Source/WebCore/page/Frame.cpp:542
> +    // Only top frame being printed should be fit to page
> +    // size. Subframes should be constrained by parents only.
> +    return m_doc->printing() && (!tree()->parent() || !tree()->parent()->m_doc->printing());

This is the OCD in me;  To clean up this comment please move "size." that starts the second line up to the end of the first line

[Vitaly Buka]

Created attachment 145853 [details]
Regression fix for Reader printing.

[Vitaly Buka]

Comment on attachment 145717 [details]
Regression fix for Reader printing.

View in context: https://bugs.webkit.org/attachment.cgi?id=145717&action=review

>> Source/WebCore/page/Frame.cpp:542
>> +    return m_doc->printing() && (!tree()->parent() || !tree()->parent()->m_doc->printing());
> 
> This is the OCD in me;  To clean up this comment please move "size." that starts the second line up to the end of the first line

Done

[Darin Adler]

Committed r119548: <http://trac.webkit.org/changeset/119548>