Summary: | printing does not use minimum page zoom factor | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ap, bdakin, cgarcia, commit-queue, darin, hyatt, mitz, mrobinson, simon.fraser, thorton | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 149368, 149595 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Ojan Vafai
2013-01-31 11:10:13 PST
*** Bug 144494 has been marked as a duplicate of this bug. *** This is because WebKitTestRunner always takes the snapshots from the UI process (expect for IOS), so in the case of printing, the layout in the web view is not the expected one. Created attachment 260360 [details]
Patch
Created attachment 260361 [details]
Try to fix CG build
Ping reviewers Comment on attachment 260361 [details] Try to fix CG build View in context: https://bugs.webkit.org/attachment.cgi?id=260361&action=review Looks OK to me. > Source/WebCore/page/PrintContext.cpp:320 > +int PrintContext::numberOfPages(Frame* frame, const FloatSize& pageSizeInPixels) Should take a Frame&. The caller is the one that checks for null. > Source/WebCore/page/PrintContext.cpp:329 > +void PrintContext::spoolAllPagesWithBoundaries(Frame* frame, GraphicsContext& graphicsContext, const FloatSize& pageSizeInPixels) Should take a Frame&. The caller is the one that checks for null. > Source/WebCore/page/PrintContext.cpp:347 > +#if PLATFORM(COCOA) > graphicsContext.translate(0, totalHeight); > graphicsContext.scale(FloatSize(1, -1)); > +#endif Seems bizarre that this is here. The platform layer is responsible for abstracting differences like this. > Source/WebCore/page/PrintContext.cpp:357 > +#if PLATFORM(COCOA) > + int boundaryLineY = currentHeight; > +#else > + int boundaryLineY = currentHeight - 1; > +#endif Same comment about the platform layer. (In reply to comment #6) > Comment on attachment 260361 [details] > Try to fix CG build > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260361&action=review > > Looks OK to me. Thanks for the review. > > Source/WebCore/page/PrintContext.cpp:320 > > +int PrintContext::numberOfPages(Frame* frame, const FloatSize& pageSizeInPixels) > > Should take a Frame&. The caller is the one that checks for null. Will change that and the all the callers. > > Source/WebCore/page/PrintContext.cpp:329 > > +void PrintContext::spoolAllPagesWithBoundaries(Frame* frame, GraphicsContext& graphicsContext, const FloatSize& pageSizeInPixels) > > Should take a Frame&. The caller is the one that checks for null. Same here. > > Source/WebCore/page/PrintContext.cpp:347 > > +#if PLATFORM(COCOA) > > graphicsContext.translate(0, totalHeight); > > graphicsContext.scale(FloatSize(1, -1)); > > +#endif > > Seems bizarre that this is here. The platform layer is responsible for > abstracting differences like this. I was surprised as well, I guess this is specific to the printing code in mac. > > Source/WebCore/page/PrintContext.cpp:357 > > +#if PLATFORM(COCOA) > > + int boundaryLineY = currentHeight; > > +#else > > + int boundaryLineY = currentHeight - 1; > > +#endif > > Same comment about the platform layer. Created attachment 261373 [details]
Patch for landing
Committed r189908: <http://trac.webkit.org/changeset/189908> This broke "run-webkit-tests --pixel" behavior, at least on Mac. How it is supposed to work is that --pixel has no effect on tests that call testRunner.dumpAsText() or testRunner.dumpAsText(false). But now, WebKitTestRunner dumps pixels for these, so all text-only tests fail when run in this mode. This breaks an important testing scenario, so I'll roll out for now. Re-opened since this is blocked by bug 149368 If the fix is straightforward, please feel free to re-land without a new review round. (In reply to comment #10) > This broke "run-webkit-tests --pixel" behavior, at least on Mac. How it is > supposed to work is that --pixel has no effect on tests that call > testRunner.dumpAsText() or testRunner.dumpAsText(false). But now, > WebKitTestRunner dumps pixels for these, so all text-only tests fail when > run in this mode. > > This breaks an important testing scenario, so I'll roll out for now. Oh, I'm sorry about this, I got confused with all the shouldDumpPixels in TestInvocation, TestRunner, InjectedBundle and InjectedBundlePage. Now that I understand it, I'm going to split the patch, first to avoid creating snapshots in the web process that will be ignored, and then the one to fix the printing snapshots. Created attachment 262050 [details]
Patch for landing
Committed r190306: <http://trac.webkit.org/changeset/190306> |