Bug 52473 - Auto-scaling to avoid orphans is broken, remove dysfunctional code
Summary: Auto-scaling to avoid orphans is broken, remove dysfunctional code
Alias: None
Product: WebKit
Classification: Unclassified
Component: Printing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.6
: P1 Normal
Assignee: Alexey Proskuryakov
Keywords: InRadar, Regression
Depends on:
Reported: 2011-01-14 13:17 PST by Alexey Proskuryakov
Modified: 2011-01-22 01:23 PST (History)
2 users (show)

See Also:

test case (302 bytes, text/html)
2011-01-14 13:17 PST, Alexey Proskuryakov
no flags Details
proposed un-fix (3.47 KB, patch)
2011-01-14 14:31 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
a more complete un-fix (4.30 KB, patch)
2011-01-14 14:34 PST, Alexey Proskuryakov
darin: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2011-01-14 13:17:20 PST
Created attachment 78986 [details]
test case

WebHTMLView attempts to scale the document down a little if it sees that the last page is very small, in order to avoid orphans. This got broken with r67771, because PrintContext::computePageRectsWithPageSizeInternal always returns all pages with the same size now.

I tried to fix this by undoing the part of PrintContext change that changed the height of the last page, but that created a bigger problem - css-page-break is no longer respected properly, as computePageRectsWithPageSizeInternal() is called after layout.
Comment 1 Alexey Proskuryakov 2011-01-14 14:31:53 PST
Created attachment 78999 [details]
proposed un-fix

The idea of this auto-shrinking is that it
(1) doesn't change page width, and
(2) is fast to try, because if we try and fail, we end up switching scale three times.

A difficulty with mechanical zooming is that it would not respect CSS page breaks, which is unacceptable. But there is no fast way to just change available page height with the new pagination model.

This feature was questionable at best (what if I print ten separate documents with identical settings - getting different font size is terribly unwelcome). I propose retiring it.
Comment 2 Alexey Proskuryakov 2011-01-14 14:34:49 PST
Created attachment 79000 [details]
a more complete un-fix
Comment 3 Darin Adler 2011-01-17 13:12:14 PST
Iā€™m not sure we should remove this. Since we broke it we need to do something. But maybe that something is to accomplish the same effect an entirely different way.
Comment 4 Alexey Proskuryakov 2011-01-17 13:15:37 PST
Comment 5 Darin Adler 2011-01-20 13:04:23 PST
Comment on attachment 79000 [details]
a more complete un-fix

We may have to re-add this feature some day, but this code is currently broken and so we should not keep it.
Comment 6 Alexey Proskuryakov 2011-01-20 14:20:38 PST
Renaming to make it clear what's actually being done here.
Comment 7 WebKit Commit Bot 2011-01-20 14:47:24 PST
Comment on attachment 79000 [details]
a more complete un-fix

Rejecting attachment 79000 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'la..." exit_code: 1

Last 500 characters of output:
er     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 76282 = eaa44b02d4cae7a82d79c083a6f7d0a2aeef9194
r76283 = 5852300ad82a05c75c47c7e9b307e8301d60a013
r76284 = 55e47eadeee8ef43d67bba22f464c20964e83d56
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://queues.webkit.org/results/7560249
Comment 8 Alexey Proskuryakov 2011-01-22 01:23:46 PST
Committed <http://trac.webkit.org/changeset/76435>.