Bug 68614

Summary: [chromium] matchMedia('print') queries fire multiple times when printing
Product: WebKit Reporter: Dominic Cooney <dominicc>
Component: PrintingAssignee: Dominic Cooney <dominicc>
Status: RESOLVED WONTFIX    
Severity: Normal CC: dglazkov, fishd, tkent, vandebo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Repro
none
WIP Patch
none
Patch
none
Patch ojan: review+, dominicc: commit-queue-

Dominic Cooney
Reported 2011-09-22 06:06:47 PDT
Created attachment 108320 [details] Repro This was first reported as a Chromium bug <http://code.google.com/p/chromium/issues/detail?id=85013> however I believe this is a bug in the Chromium port of WebKit so refiling it here. Repro steps: 1. Open the attached matchmedia.html (from the Chromium bug.) 2. File > Print 3. Print the document/generate and view a PDF Expected printed output: Listener triggered! media: print matches: true addListener: function addListener() { [native code] } removeListener: function removeListener() { [native code] } What is actually printed: Listener triggered! media: print matches: true addListener: function addListener() { [native code] } removeListener: function removeListener() { [native code] } Listener triggered! media: print matches: false addListener: function addListener() { [native code] } removeListener: function removeListener() { [native code] } Listener triggered! media: print matches: true addListener: function addListener() { [native code] } removeListener: function removeListener() { [native code] } The matchMedia callback is being triggered too often. I believe the problem stems from Chromium’s WebFrame creating and destroying a PrintContext when it generates the print preview, and again when the document is printed. It should probably reuse one PrintContext.
Attachments
Repro (353 bytes, text/html)
2011-09-22 06:06 PDT, Dominic Cooney
no flags
WIP Patch (11.37 KB, patch)
2011-09-22 06:12 PDT, Dominic Cooney
no flags
Patch (11.34 KB, patch)
2011-10-24 01:49 PDT, Dominic Cooney
no flags
Patch (11.52 KB, patch)
2011-10-24 15:12 PDT, Dominic Cooney
ojan: review+
dominicc: commit-queue-
Dominic Cooney
Comment 1 2011-09-22 06:12:39 PDT
Created attachment 108322 [details] WIP Patch
WebKit Review Bot
Comment 2 2011-09-22 06:34:53 PDT
Comment on attachment 108322 [details] WIP Patch Attachment 108322 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9792277
WebKit Review Bot
Comment 3 2011-09-22 08:06:21 PDT
Comment on attachment 108322 [details] WIP Patch Attachment 108322 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9801074
Dominic Cooney
Comment 4 2011-09-22 16:18:40 PDT
Expect this to fail cr-*-ews, it is an API change; updated callers <http://codereview.chromium.org/7993005/>
Dominic Cooney
Comment 5 2011-10-24 01:49:27 PDT
WebKit Review Bot
Comment 6 2011-10-24 01:51:13 PDT
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Dominic Cooney
Comment 7 2011-10-24 01:52:09 PDT
Comment on attachment 112162 [details] Patch CQ-: this should not be landed until the Chromium side is reviewed; see <http://crbug.com/85013>.
WebKit Review Bot
Comment 8 2011-10-24 01:54:24 PDT
Comment on attachment 112162 [details] Patch Attachment 112162 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10202732
Steve VanDeBogart
Comment 9 2011-10-24 13:53:39 PDT
Drive by comment: WebFrame.h:427 Out of date comment? It looks like printBegin no longer actually reformats the page (for good reason). Now you must call setPageSizeResolution() at least once to set the other parameters and reflow the page.
Dominic Cooney
Comment 10 2011-10-24 15:12:45 PDT
Dominic Cooney
Comment 11 2011-10-24 15:14:04 PDT
(In reply to comment #9) > Drive by comment: WebFrame.h:427 Out of date comment? It looks like printBegin no longer actually reformats the page (for good reason). Now you must call setPageSizeResolution() at least once to set the other parameters and reflow the page. Good point. I updated the comments.
WebKit Review Bot
Comment 12 2011-10-24 15:34:11 PDT
Comment on attachment 112260 [details] Patch Attachment 112260 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10201936
Ojan Vafai
Comment 13 2012-04-19 16:35:51 PDT
Comment on attachment 112260 [details] Patch Patch looks fine, but you need to fix webkit/glue to work with the new code first (note that cr-linux didn't compile). Two sided patches aren't allowed (i.e. the chromium tip-of-tree webkit build should never be broken).
Dominic Cooney
Comment 14 2012-06-01 09:37:19 PDT
The conclusion on the Chromium bug was that firing fewer media queries would require making the window less interactive (eg not resizing) when print preview was active.
Note You need to log in before you can comment on or make changes to this bug.