Merge pull request #104 from marp-team/fix-printable-selector

Fix over-scoped selectors injected by printable plugin

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixed
 
 - Revert resolutions for `ajv` ([#102](https://github.com/marp-team/marpit/pull/102))
+- Fix over-scoped selectors injected by printable plugin ([#104](https://github.com/marp-team/marpit/pull/104))
 
 ### Added
 

src/postcss/printable.js

@@ -11,20 +11,19 @@ import postcss from 'postcss'
  * @param {string} opts.height
  * @alias module:postcss/printable
  */
-const plugin = postcss.plugin('marpit-postcss-printable', opts => css =>
+const plugin = postcss.plugin('marpit-postcss-printable', opts => css => {
+  css.walkAtRules('media', rule => {
+    if (rule.params === 'marpit-print') rule.remove()
+  })
+
   css.first.before(
     `
 @page {
   size: ${opts.width} ${opts.height};
   margin: 0;
 }
 
-@media print {
-  html, body {
-    margin: 0;
-    page-break-inside: avoid;
-  }
-
+@media marpit-print {
   section {
     page-break-before: always;
   }
@@ -42,6 +41,22 @@ const plugin = postcss.plugin('marpit-postcss-printable', opts => css =>
 }
 `.trim()
   )
+})
+
+/**
+ * The post-process PostCSS plugin of Marpit printable plugin.
+ *
+ * @alias module:postcss/printable.postprocess
+ */
+export const postprocess = postcss.plugin(
+  'marpit-postcss-printable-postprocess',
+  () => css =>
+    css.walkAtRules('media', rule => {
+      if (rule.params !== 'marpit-print') return
+
+      rule.params = 'print'
+      rule.first.before('html, body { margin: 0; page-break-inside: avoid; }')
+    })
 )
 
 export default plugin

src/theme_set.js

@@ -4,7 +4,9 @@ import postcssImportReplace from './postcss/import/replace'
 import postcssImportRollup from './postcss/import/rollup'
 import postcssImportSuppress from './postcss/import/suppress'
 import postcssPagination from './postcss/pagination'
-import postcssPrintable from './postcss/printable'
+import postcssPrintable, {
+  postprocess as postcssPrintablePostProcess,
+} from './postcss/printable'
 import postcssPseudoPrepend from './postcss/pseudo_selector/prepend'
 import postcssPseudoReplace from './postcss/pseudo_selector/replace'
 import Theme from './theme'
@@ -213,6 +215,7 @@ class ThemeSet {
         postcssPagination,
         postcssPseudoPrepend,
         postcssPseudoReplace(opts.containers, slideElements),
+        opts.printable && postcssPrintablePostProcess,
         postcssImportRollup,
       ].filter(p => p)
     )

test/_supports/postcss_finder.js

@@ -0,0 +1,24 @@
+export function find(from, cond) {
+  let found
+  return (
+    from.some(rule => {
+      for (const key of Object.keys(cond)) {
+        if (rule[key] === undefined) return false
+        if (typeof cond[key] === 'function') {
+          if (!cond[key](rule[key])) return false
+        } else if (rule[key] !== cond[key]) {
+          return false
+        }
+      }
+      found = rule
+      return true
+    }) && found
+  )
+}
+
+export const findAtRule = (f, cond) => find(f, { type: 'atrule', ...cond })
+export const findComment = (f, cond) => find(f, { type: 'comment', ...cond })
+export const findDecl = (f, cond) => find(f, { type: 'decl', ...cond })
+export const findRule = (f, cond) => find(f, { type: 'rule', ...cond })
+
+export default find

test/postcss/printable.js

@@ -1,36 +1,49 @@
+import dedent from 'dedent'
 import postcss from 'postcss'
-import printable from '../../src/postcss/printable'
+import { findAtRule, findDecl, findRule } from '../_supports/postcss_finder'
+import printable, { postprocess } from '../../src/postcss/printable'
 
 describe('Marpit PostCSS printable plugin', () => {
   const run = (input, opts) =>
-    postcss([printable(opts)]).process(input, { from: undefined })
+    postcss([printable(opts), postprocess]).process(input, { from: undefined })
 
   it('prepends style for printing', () => {
-    const css = 'section.theme { background: #fff; }'
+    const css = dedent`
+      section.theme { background: #fff; }
+
+      @media marpit-print {
+        /* Declarations for internal will remove */
+        html { margin: 10px; }
+      }
+    `
+
     const opts = { width: '640px', height: '480px' }
 
-    return run(css, opts).then(result => {
-      const rules = []
-      result.root.walk(node => {
-        if (
-          node.type === 'atrule' ||
-          (node.type === 'rule' && node.selector === 'section.theme')
-        )
-          rules.push(node)
+    return run(css, opts).then(({ root }) => {
+      const page = findAtRule(root, { name: 'page' })
+      const print = findAtRule(root, { name: 'media', params: 'print' })
+      const marpitPrint = findAtRule(root, {
+        name: 'media',
+        params: 'marpit-print',
       })
 
-      expect(rules).toStrictEqual([
-        expect.objectContaining({ type: 'atrule', name: 'page' }),
-        expect.objectContaining({ type: 'atrule', name: 'media' }),
-        expect.objectContaining({ type: 'rule', selector: 'section.theme' }),
-      ])
+      expect(page).toBeTruthy()
+      expect(print).toBeTruthy()
+      expect(marpitPrint).toBeFalsy()
 
-      let sizeDecl
-      result.root.walkDecls('size', rule => {
-        sizeDecl = rule
-      })
+      // @page at-rule
+      const pageSizeDecl = findDecl(page, { prop: 'size' })
+      expect(pageSizeDecl.value).toBe('640px 480px')
+
+      // @print at-rule for print
+      for (const tag of ['html', 'body']) {
+        const r = findRule(print, { selectors: s => s.includes(tag) })
+        expect(findDecl(r, { prop: 'page-break-inside' }).value).toBe('avoid')
+        expect(findDecl(r, { prop: 'margin' }).value).toBe('0')
+      }
 
-      expect(sizeDecl.value).toBe('640px 480px')
+      // Original CSS
+      expect(findRule(root, { selector: 'section.theme' })).toBeTruthy()
     })
   })
 })

test/theme_set.js

@@ -276,6 +276,27 @@ describe('ThemeSet', () => {
 
         expect(css).not.toContain('INVALID')
       })
+
+      it('cannot apply unscoped rules by using @media print', () => {
+        const pack = before =>
+          instance.pack(undefined, { before, printable: true })
+
+        // `@media print` will apply scope to defined rules.
+        const print = '@media print { body { background: red; } }'
+        const printCSS = pack(print)
+
+        expect(printCSS.split('@media print').length - 1).toBe(2)
+        expect(printCSS).toContain(
+          '@media print { section body { background: red; } }'
+        )
+
+        // `@media marpit-print` internal at-rule will remove.
+        const marpitPrint = '@media marpit-print { body { background: red; } }'
+        const marpitPrintCSS = pack(marpitPrint)
+
+        expect(marpitPrintCSS.split('@media print').length - 1).toBe(1)
+        expect(marpitPrintCSS).not.toContain('background: red;')
+      })
     })
   })
 })