New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 664235 link

Starred by 33 users

Issue metadata

Status: Fixed
Owner:
Use other robhogan account instead.
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Printing is breaking pages in the middle of images

Reported by cda...@gmail.com, Nov 10 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36

Example URL:
mtgpress.net

Steps to reproduce the problem:
1. Go to http://mtgpress.net/

2. Paste in the contents of the following pastebin URL, then click 'Build':
https://bpaste.net/raw/e0e3f61217d5

3. Print the resulting page

What is the expected behavior?
Images should not be split across pages when they don't need to be.  On previous Chrome versions images were not split across pages. 

What went wrong?
Images are split across pages.  This is ugly, and in this case, doing so ruins the functionality of the site.

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? Yes Unsure

Does this work in other browsers? Yes

Chrome version: 54.0.2840.71  Channel: n/a
OS Version: OS X 10.11.6
Flash Version: Shockwave Flash 23.0 r0
 
Screen Shot 2016-11-10 at 1.24.06 PM.png
1.3 MB View Download
Components: UI>Browser>PrintPreview
Labels: -Pri-2 -Type-Compat -OS-Mac M-56 has-Bisect OS-Linux OS-Windows Pri-1 Type-Bug-Regression
Owner: msten...@opera.com
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on Windows 10 and Ubuntu 14.04 using chrome reported version #54.0.2840.71 but unable to reproduce the same in latest canary #56.0.2916.0.

Note: Unable to reproduce the issue in Mac 10.11.16 using both chrome reported version #54.0.2840.71 and 
latest canary #56.0.2916.0.

Reverse Bisect Information:
=====================
Good build: 56.0.2916.0  Revision(431463)
Bad Build : 56.0.2914.0	 Revision(430837)

Unable to provide the CL by running the bisect tool as all the builds that were triggered were bad builds.
Hence, providing the CL from Omahaproxy.

Change Log URL(Omahaproxy): 
https://chromium.googlesource.com/chromium/src/+log/56.0.2914.0..56.0.2916.0?pretty=fuller&n=10000

From the above change log suspecting below change

Review url: https://codereview.chromium.org/2486413002

mstensho@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks...!!

Comment 2 by msten...@opera.com, Nov 14 2016

Reproduced. Reverting that suspected CL makes no difference, though.

Comment 3 by msten...@opera.com, Nov 14 2016

Not sure what happened with that bisect. Maybe the issue got fixed temporarily before it broke again. This is not a very recent regression, i.e. not Chrome 56. The original reporter claims this to be present in Chrome 54, and indeed, I can reproduce it in Opera 41, which uses Chrome 54.

Comment 4 by msten...@opera.com, Nov 14 2016

tc.html
2.6 KB View Download

Comment 5 by msten...@opera.com, Nov 15 2016

This has something to do with page scaling. There's a width:105% declaration, which is never going to fit on a page, no matter how much we scale down.

It almost look like we make a few attempts at scaling, and then finally give up at some scale factor, but forget to lay out the document one final time (to get the page breaks right).
tc-simpler.html
831 bytes View Download

Comment 6 by msten...@opera.com, Feb 8 2017

Cc: nainar@chromium.org
@nainar - no media queries here, but isn't it still the same as  bug 660058 ?

Comment 7 by robho...@gmail.com, Mar 12 2017

Cc: msten...@opera.com thestig@chromium.org robhogan@chromium.org
 Issue 690621  has been merged into this issue.

Comment 8 by msten...@opera.com, Mar 13 2017

If someone who knows how this works in the non-Blink parts of printing / print preview, any help would be appreciated. I tried to investigate this some time ago, but I didn't really understand much, apart from the fact that we print/paint at a different (larger) page size than what we've laid out.
I made some good progress on this recently. I have a CL up at https://codereview.chromium.org/2747623002/ that I need to get a test working for.

If we need to keep the current position of ResizeForPrinting() than what we want to do is fix forceLayoutForPagination() - it sets layout overflow that inadvertently gets cleared again by adjustViewSizeAndLayout() and that results in the bug here and 690621. 

It doesn't fix another couple of variant bugs caused by https://codereview.chromium.org/2089373002 however - so I think reverting that CL is still the right thing to do unless we can show that reverting it causes the transition issues it was aiming to solve to re-appear - I can't recreate them even with it reverted.

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/593eebedc4013ea212b05c21dba300d41a6d7977

commit 593eebedc4013ea212b05c21dba300d41a6d7977
Author: robhogan <robhogan@gmail.com>
Date: Wed Mar 29 05:45:24 2017

Avoid clearing a page's layout overflow after paginated layout

This was flushed out by https://codereview.chromium.org/2089373002.

If we've forced a layout on paged media and find that our width exceeds
the page's then we may end up needing to clip some of the horizontal overflow.
We do this by calculating it manually and storing it as layout overflow.
However if our scrollbars are changing we will force another layout on the
frame that overwrites this good work. So make sure we preserve any clipped
overflow on the frame.

It's very hard to reproduce print preview issues reliably but fortunately
this specific issue can be recreated during layout testing by using
@media page rules so that's how we reproduce it here. I was going to revert
the CL in 2089373002 but it's very hard to prove either way that that is
achieving anything - this change on the other hand has an intelligible
rationale and does ensure the expected clipping of width.

Related bugs potentially fixed by this are 660058 and 690621.

BUG= 664235 

Review-Url: https://codereview.chromium.org/2747623002
Cr-Commit-Position: refs/heads/master@{#460292}

[add] https://crrev.com/593eebedc4013ea212b05c21dba300d41a6d7977/third_party/WebKit/LayoutTests/printing/respect-layout-overflow-from-pagination-expected.html
[add] https://crrev.com/593eebedc4013ea212b05c21dba300d41a6d7977/third_party/WebKit/LayoutTests/printing/respect-layout-overflow-from-pagination.html
[modify] https://crrev.com/593eebedc4013ea212b05c21dba300d41a6d7977/third_party/WebKit/Source/core/frame/FrameView.cpp

Comment 11 by msten...@opera.com, Mar 29 2017

Owner: robhogan@chromium.org
Status: Fixed (was: Assigned)
Awesome! Thanks, Rob!

Comment 12 by msten...@opera.com, Apr 20 2017

Cc: jmukthavaram@chromium.org soushi@chromium.org kkaluri@chromium.org brajkumar@chromium.org hdodda@chromium.org
 Issue 660058  has been merged into this issue.

Comment 13 by ajha@chromium.org, May 9 2017

 Issue 704494  has been merged into this issue.

Sign in to add a comment