RESOLVED FIXED 172361
matchMedia('print').addListener() fires in WK1 but never in WK2 when printing (breaks printing Google maps, QuickLooks) 
https://bugs.webkit.org/show_bug.cgi?id=172361
Summary matchMedia('print').addListener() fires in WK1 but never in WK2 when printing...
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+
DetailsFormatted DiffDiff
patch (5.94 KB, patch)
2017-05-21 05:31 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
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.