Bug 172361

Summary:

matchMedia('print').addListener() fires in WK1 but never in WK2 when printing (breaks printing Google maps, QuickLooks)

Product:

WebKit

Reporter:

Antti Koivisto <koivisto>

Component:

Printing

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, commit-queue, sam

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
patch	sam: review+
patch	none

Antti Koivisto

Reported 2017-05-19 09:31:21 PDT

<html><body> <div id="main">Before print</div> <script> var mql = window.matchMedia("print"); mql.addListener(onPrint); function onPrint(e) { if (e.matches) { document.getElementById("main").innerHTML = "Printing"; } else { document.getElementById("main").innerHTML = "After print"; } } </script></body></html>

Attachments
patch (5.70 KB, patch) 2017-05-19 09:42 PDT, Antti Koivisto	sam: review+	Details Formatted Diff Diff
patch (5.94 KB, patch) 2017-05-21 05:31 PDT, Antti Koivisto	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Antti Koivisto

Comment 1 2017-05-19 09:31:43 PDT

<rdar://problem/28777408>

Antti Koivisto

Comment 2 2017-05-19 09:42:28 PDT

Created attachment 310665 [details] patch

Sam Weinig

Comment 3 2017-05-19 10:01:58 PDT

Comment on attachment 310665 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=310665&action=review > Source/WebCore/ChangeLog:14 > + (WebCore::FrameView::layout): > + > + Evaluate matchMedia queries unconditionally. No idea why it wasn't like that. Out of curiosity, what made this work in WK1 but not WK2?

Antti Koivisto

Comment 4 2017-05-19 10:06:55 PDT

If anything triggers an additional layout the bug goes away. I think WK1 worked by just by having different printing code that ended up doing more layouts. I had to add a clean printing simulation to Internals to test this.

Alexey Proskuryakov

Comment 5 2017-05-19 10:14:50 PDT

Comment on attachment 310665 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=310665&action=review > Source/WebCore/testing/Internals.h:348 > + void setPrinting(int width, int height); What resets m_printContext between tests? > LayoutTests/fast/media/matchMedia-print.html:15 > +if (window.internals) > + window.internals.setPrinting(500,500); Is it doable with one of the existing print testing functions? There are some in Internals (search for PeintContext in Internals.cpp), and there is setPrinting in TestRunner too.

Antti Koivisto

Comment 6 2017-05-19 10:54:03 PDT

> What resets m_printContext between tests? Internals gets recreated for each test. Lifetime of m_printContext might still be problematic as GC could keep Internals alive too long. I'll look into it. > Is it doable with one of the existing print testing functions? There are > some in Internals (search for PeintContext in Internals.cpp), and there is > setPrinting in TestRunner too. No, as mentioned above and in the ChangeLog.

Antti Koivisto

Comment 7 2017-05-21 05:31:56 PDT

Created attachment 310806 [details] patch

WebKit Commit Bot

Comment 8 2017-05-21 08:08:21 PDT

Comment on attachment 310806 [details] patch Clearing flags on attachment: 310806 Committed r217197: <http://trac.webkit.org/changeset/217197>

WebKit Commit Bot

Comment 9 2017-05-21 08:08:23 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.