diff --git a/docs/screenshots/README.md b/docs/screenshots/README.md index ca184bb..16663f7 100644 --- a/docs/screenshots/README.md +++ b/docs/screenshots/README.md @@ -44,6 +44,25 @@ browser's other furniture is on the screen. | `wonderink-packaged` | the packaged shell against the real app.wonderink.org, launched from the `.deb` on a rebooted machine | | `wonderink-disabled` | `WONDERINK_ENABLED=false` — no lesson button, the browser and the disc player where they were | +## Wonder Ink worksheets + +0.2.2. A lesson link that downloads a PDF comes back as a tab, and +nothing else about the room's locks moves. + +| | | +| --- | --- | +| `pdf-opened` | the worksheet a click downloaded, open as a tab beside the lesson and actually rendered — three pages, not a blank frame | +| `pdf-page2` | page 2 of 3: multi-page navigation inside the tab | +| `pdf-spaces` | `Leader Guide, Week 1 (Advent).pdf` — spaces, a comma and brackets, through the download and into a tab | +| `pdf-print` | printing the worksheet, on the room's own Elmer queue | +| `pdf-closed-then-page` | the worksheet tab closed, the lesson still there and still working, and an ordinary `target=_blank` tab beside it | +| `pdf-broken` | HTML the server called `.pdf`: refused on its contents, with a message, and no blank tab | +| `pdf-interrupted` | a download that did not finish, saying so. The first version of this said nothing at all | +| `pdf-guard` | three attempts to reach a local file from the page — a link, a script, and a link to a worksheet that was downloaded but not opened by the shell. All refused | +| `pdf-dotmenu` | the whole of the PDF viewer's overflow menu, audited: two-page view, annotations, present, properties. No open, no save-as, no external link | +| `pdf-after-save` | the viewer's own download button: straight to Wonder Ink's directory with a notification, no file chooser and no duplicate tab | +| `pdf-autoplay-preserved` | the 0.2.1 Present-view autoplay fix, still working on the packaged 0.2.2 | + ## Parent Call | | | diff --git a/docs/screenshots/pdf-after-save.png b/docs/screenshots/pdf-after-save.png new file mode 100644 index 0000000..c022b96 Binary files /dev/null and b/docs/screenshots/pdf-after-save.png differ diff --git a/docs/screenshots/pdf-autoplay-preserved.png b/docs/screenshots/pdf-autoplay-preserved.png new file mode 100644 index 0000000..84db183 Binary files /dev/null and b/docs/screenshots/pdf-autoplay-preserved.png differ diff --git a/docs/screenshots/pdf-broken.png b/docs/screenshots/pdf-broken.png new file mode 100644 index 0000000..427edfc Binary files /dev/null and b/docs/screenshots/pdf-broken.png differ diff --git a/docs/screenshots/pdf-closed-then-page.png b/docs/screenshots/pdf-closed-then-page.png new file mode 100644 index 0000000..2e1b40f Binary files /dev/null and b/docs/screenshots/pdf-closed-then-page.png differ diff --git a/docs/screenshots/pdf-dotmenu.png b/docs/screenshots/pdf-dotmenu.png new file mode 100644 index 0000000..90aac5b Binary files /dev/null and b/docs/screenshots/pdf-dotmenu.png differ diff --git a/docs/screenshots/pdf-guard.png b/docs/screenshots/pdf-guard.png new file mode 100644 index 0000000..0bbabb4 Binary files /dev/null and b/docs/screenshots/pdf-guard.png differ diff --git a/docs/screenshots/pdf-interrupted.png b/docs/screenshots/pdf-interrupted.png new file mode 100644 index 0000000..cf9a70b Binary files /dev/null and b/docs/screenshots/pdf-interrupted.png differ diff --git a/docs/screenshots/pdf-opened.png b/docs/screenshots/pdf-opened.png new file mode 100644 index 0000000..8c482e8 Binary files /dev/null and b/docs/screenshots/pdf-opened.png differ diff --git a/docs/screenshots/pdf-page2.png b/docs/screenshots/pdf-page2.png new file mode 100644 index 0000000..09f6c54 Binary files /dev/null and b/docs/screenshots/pdf-page2.png differ diff --git a/docs/screenshots/pdf-print.png b/docs/screenshots/pdf-print.png new file mode 100644 index 0000000..5e00516 Binary files /dev/null and b/docs/screenshots/pdf-print.png differ diff --git a/docs/screenshots/pdf-spaces.png b/docs/screenshots/pdf-spaces.png new file mode 100644 index 0000000..5f9ee88 Binary files /dev/null and b/docs/screenshots/pdf-spaces.png differ diff --git a/docs/wonder-ink.md b/docs/wonder-ink.md index b5667b4..8c94d33 100644 --- a/docs/wonder-ink.md +++ b/docs/wonder-ink.md @@ -76,6 +76,112 @@ cut/copy/paste in a text field, print. Keys: Ctrl+W closes a tab, Alt+Left and Alt+Right navigate, Ctrl+P prints, Ctrl+Tab switches. Ctrl+T, Ctrl+N and Ctrl+L are deliberately unbound. +## Worksheets + +Some of Wonder Ink's lesson links are a download rather than a page: the +server sends the PDF as an attachment, so there is nothing for a tab to +show and the file simply lands on disk. + +In this room that was the same as it not arriving at all. There is no file +manager, no application launcher and no downloads bar, on purpose, and a +teacher is not going to open a maintenance terminal to print a Christmas +tree. 0.2.1 downloaded the worksheet, said so in a notification, and left +it somewhere unreachable. + +So a worksheet that finishes downloading is opened in a tab: + +1. a teacher clicks a worksheet link; +2. Wonder Ink downloads it into `~/Downloads/Wonder Ink`; +3. when the download **completes**, and the file really is a PDF, it + opens as another tab, named after the file; +4. the teacher reads it, scrolls it, prints it to the room's printer; +5. closing that tab leaves the lesson exactly where it was. + +Rendering is the engine's own PDF viewer — the same PDFium that Chrome +uses — so there is no second rendering library in the room and no +separate application. `QtWebEngineSettings.PdfViewerEnabled` was already +on for remote PDFs; what 0.2.2 adds is opening a *local* one, which was +verified against a real completed file rather than assumed from the +remote case. + +### What is refused, and why + +A completed download is not the same as a usable worksheet, and each of +these was a real case in the lab: + +| | | +| --- | --- | +| an interrupted download | the notification says it did not finish and to try again. Nothing opens | +| HTML that the server called `.pdf` | refused on its first five bytes, not on its name. Without this the teacher gets a blank tab and no reason | +| an empty or unreadable file | the same | +| anything that is not a PDF | saved, and said so, exactly as before. Nothing is opened and nothing is executed | + +The terminal state is read from `stateChanged`, not `isFinishedChanged`. +Measured: an interrupted download never emits `isFinishedChanged` at all, +so the first version of this said nothing whatsoever about a worksheet +that failed to arrive — which is the one case a teacher most needs told +about. + +### Which local files Wonder Ink will open + +Exactly the ones it opened itself, and nothing else. + +Opening a worksheet adds that **one resolved path** to a list. The page's +`acceptNavigationRequest` refuses every `file:` URL that is not on that +list, so a lesson cannot navigate to one, a link cannot point at one and a +redirect cannot reach one. Chromium already refuses to let a remote page +reach `file:` at all — verified in the lab, where a link, a +`location.href` and a link to a worksheet that *had* been downloaded but +not opened by the shell were all refused — so this is the second lock +rather than the only one. It is there because the first one is somebody +else's. + +Alongside it, two Qt defaults are turned off rather than left implicit: +`LocalContentCanAccessFileUrls` (on by default, and would let a local +document read other local documents) and `LocalContentCanAccessRemoteUrls`. + +The print dialog's **Print to File** option is also removed. Choosing it +enables an output path with a "..." button, which is a file chooser, in a +room whose whole design is that there is no file chooser. + +### What a teacher still cannot reach + +**Anything that is not a PDF.** A lesson handing out a Word document, a +spreadsheet, a zip of images or a PowerPoint still downloads, still says +so, and still lands somewhere a teacher has no way to open. This release +deliberately fixes only the worksheet case, because that is the one the +room actually hit and because "open whatever arrives" is how an appliance +stops being one. + +The obvious small follow-ups, in the order they are worth doing: + +1. **Presentations.** Already planned separately, and the larger of the + two jobs — a PowerPoint needs a renderer, not just a viewer. +2. **A worksheets list.** Not a file manager: one Wonder Ink tab, showing + the PDFs already in `~/Downloads/Wonder Ink`, each opening in a tab the + same way a fresh download does. It reads one directory, offers no + navigation out of it, and needs no new permission — the paths it offers + are ones the shell would open anyway. That covers "a teacher closed the + worksheet tab and wants it back", which is the next thing that will be + asked for. + +Neither is in this release. + +### Where Wonder Ink's downloads end, and Chrome's begin + +They never meet, and not because anything inspects a filename. + +The download handler is connected to **Wonder Ink's own profile**, so it +only ever sees downloads that started in a Wonder Ink tab. Chrome is a +separate application with a separate profile and its own download +handling, and nothing here changes, watches or intercepts it. There is no +directory watcher anywhere in the classroom: a PDF that Chrome downloads +is Chrome's business and stays that way. + +`~/Downloads/Wonder Ink` is a directory of its own inside the ordinary +Downloads folder. That is partly tidiness and mostly the security +argument: it is the only place a worksheet can come from. + ## What stayed the same * **A separate profile from the ordinary browser.** A teacher's Wonder Ink diff --git a/scripts/classroom-wonderink b/scripts/classroom-wonderink index d299ef1..a2d7dbb 100755 --- a/scripts/classroom-wonderink +++ b/scripts/classroom-wonderink @@ -79,6 +79,27 @@ CACHE_HOME = Path( PROFILE = DATA_HOME / "ekklesia-classroom" / "wonderink-shell" CACHE = CACHE_HOME / "ekklesia-classroom" / "wonderink-shell" +# Where a lesson's handouts land. +# +# A directory of Wonder Ink's own, inside the ordinary Downloads folder +# rather than beside it, and the ordinary browser is not told about it and +# does not use it. Two reasons, and the second is the one that matters: +# +# a teacher's worksheets are not mixed in with whatever the room's +# shared browser has been asked to save; and +# +# it is the whole of the answer to "which local files may Wonder Ink +# open". Nothing outside this directory is ever loaded, and the +# navigation guard in LessonPage enforces that rather than trusting it. +DOWNLOADS = Path( + os.environ.get("XDG_DOWNLOAD_DIR") or Path.home() / "Downloads" +) / "Wonder Ink" + +# What a PDF starts with. A server that answers a worksheet link with an +# HTML error page called .pdf is a real thing, and opening it would give a +# teacher a blank tab with no way to tell why. +PDF_MAGIC = b"%PDF-" + # Widevine, if this machine has it. # # Chrome is installed on every classroom machine for the ordinary @@ -102,7 +123,11 @@ if WIDEVINE.exists(): from PyQt6.QtCore import QEvent, Qt, QUrl # noqa: E402 from PyQt6.QtGui import QAction, QGuiApplication, QKeySequence # noqa: E402 from PyQt6.QtNetwork import QLocalServer, QLocalSocket # noqa: E402 -from PyQt6.QtPrintSupport import QPrintDialog, QPrinter # noqa: E402 +from PyQt6.QtPrintSupport import ( # noqa: E402 + QAbstractPrintDialog, + QPrintDialog, + QPrinter, +) from PyQt6.QtWebEngineCore import ( # noqa: E402 QWebEngineDownloadRequest, QWebEnginePage, @@ -157,6 +182,38 @@ class LessonPage(QWebEnginePage): # would otherwise fill the journal on every page load. pass + def acceptNavigationRequest(self, url, navigation_type, is_main_frame): + """Which URLs this window is willing to go to at all. + + Wonder Ink opens a teacher's downloaded worksheet by handing the + tab a file: URL, so from this release the shell loads local files + — and the moment it does, "which local files" stops being + rhetorical. This is where it is answered. + + A file: URL is accepted only if the window put that exact path on + its own list, which it does for one thing: a PDF that its own + download handler has just finished writing into DOWNLOADS. A + lesson cannot navigate to one, a link cannot point at one, and a + redirect cannot reach one, because none of those go through the + list. + + Chromium already refuses to let a remote page navigate to file:, + so this is the second lock rather than the only one. It is here + because the first one is somebody else's and this one is ours. + """ + if url.isLocalFile(): + allowed = self._window.may_open(Path(url.toLocalFile())) + + if not allowed: + say(f"refusing a local file this window did not open: {url.toLocalFile()}") + + return allowed + + # Everything a lesson legitimately does. Anything else — a mail + # client, a chooser, some handler a desktop happens to register — + # is not a thing a classroom appliance follows. + return url.scheme() in ("http", "https", "about", "data", "blob", "chrome-extension") + class LessonView(QWebEngineView): """A page, and a right-click menu with nothing dangerous on it.""" @@ -215,6 +272,14 @@ class WonderInk(QMainWindow): super().__init__() self._profile = profile self._fullscreen_view = None + # Every local file this window has decided to open, resolved. The + # navigation guard in LessonPage consults exactly this, so a path + # that is not here cannot be loaded however it is asked for. + self._openable = set() + # Tabs showing a worksheet rather than a page. Their names come + # from the file, because a PDF has no document title to take one + # from, and their downloads are not re-opened as further tabs. + self._pdf_views = {} self.setWindowTitle("Wonder Ink") @@ -274,6 +339,7 @@ class WonderInk(QMainWindow): return self._tabs.removeTab(index) + self._pdf_views.pop(view, None) view.page().deleteLater() view.deleteLater() self._show_tab_bar() @@ -291,7 +357,12 @@ class WonderInk(QMainWindow): if index < 0: return - title = (title or "Wonder Ink").strip() + # A worksheet keeps the name it was downloaded as. Chromium's PDF + # viewer leaves document.title empty, so left alone every PDF tab + # would be called "Wonder Ink" and two of them would be + # indistinguishable. + title = self._pdf_views.get(view) or (title or "Wonder Ink") + title = title.strip() short = title if len(title) <= TAB_TITLE else title[: TAB_TITLE - 1] + "…" self._tabs.setTabText(index, short) self._tabs.setTabToolTip(index, title) @@ -364,6 +435,125 @@ class WonderInk(QMainWindow): if count > 1: self._tabs.setCurrentIndex((self._tabs.currentIndex() - 1) % count) + # ---- a lesson's handouts -------------------------------------------- + + def may_open(self, path): + """Is this a local file the window itself decided to open?""" + try: + return path.resolve() in self._openable + except OSError: + return False + + def on_download(self, download): + """A lesson handing the teacher a file. + + Wonder Ink gives out leader guides and colouring pages, and some + of its worksheet links are a download rather than a page — the + server sends the PDF as an attachment, so there is nothing for a + tab to show and the file simply lands on disk. + + Which, in this room, is the same as it not arriving at all. There + is no file manager, no application launcher and no downloads bar, + on purpose, and a teacher is not going to open a maintenance + terminal to print a Christmas tree. So a worksheet that finishes + downloading is opened in a tab, which is where the teacher was + looking anyway. + """ + download.setDownloadDirectory(str(DOWNLOADS)) + download.accept() + + # Read now rather than in the callback: the request object is + # deleted once it finishes, and reading it then is a crash. + name = download.downloadFileName() + mime = download.mimeType() + # A save from the PDF viewer's own toolbar is a download whose + # page is already a worksheet. Opening the copy would give a + # teacher a second identical tab every time they pressed it. + from_worksheet = download.page() in [ + view.page() for view in self._pdf_views + ] + + # stateChanged, not isFinishedChanged. + # + # Measured: a download interrupted mid-transfer — the church wi-fi + # dropping, a server hanging up — reaches + # DownloadState.DownloadInterrupted and isFinishedChanged is never + # emitted at all. Connected to that signal, Wonder Ink said + # nothing whatsoever about a worksheet that did not arrive, which + # is the one case a teacher most needs told about. stateChanged + # fires for every terminal state, so it is the one to listen to. + states = QWebEngineDownloadRequest.DownloadState + settled = [] + + def state_changed(state): + if settled or state == states.DownloadInProgress: + return + + settled.append(state) + + if state == states.DownloadCancelled: + return + + if state == states.DownloadInterrupted: + # interruptReasonString names which of the dozen ways it + # went wrong, which is the difference between a teacher + # retrying and a teacher giving up. + notify( + "The worksheet did not finish downloading", + f"{name} — {download.interruptReasonString()}. Try again.", + ) + return + + path = DOWNLOADS / name + + if not from_worksheet and self.open_worksheet(path, mime): + return + + notify("Saved", f"{name} is in {DOWNLOADS}") + + download.stateChanged.connect(state_changed) + + def open_worksheet(self, path, mime=""): + """Open a finished PDF in a tab. True if it was.""" + if mime != "application/pdf" and path.suffix.lower() != ".pdf": + return False + + # A completed download can still be a file worth refusing: an + # error page the server called .pdf, or a nought-byte truncation. + # The tab would come up blank and say nothing about why. + try: + with open(path, "rb") as handle: + head = handle.read(len(PDF_MAGIC)) + except OSError as error: + say(f"cannot read {path}: {error}") + notify("Could not open the worksheet", f"{path.name} could not be read") + return True + + if head != PDF_MAGIC: + say(f"not a PDF: {path}") + notify( + "Could not open the worksheet", + f"{path.name} did not arrive as a PDF. It is saved in {DOWNLOADS}.", + ) + return True + + self.open_pdf(path) + return True + + def open_pdf(self, path): + """A worksheet, as a tab of its own.""" + resolved = path.resolve() + + # The one place anything is added to the list the navigation + # guard reads. Note what it is: a single resolved path, not a + # directory and not a pattern. + self._openable.add(resolved) + + view = self.open_tab() + self._pdf_views[view] = path.stem + self._retitle(view, path.stem) + view.load(QUrl.fromLocalFile(str(resolved))) + # ---- printing ------------------------------------------------------- def print_current(self): @@ -376,6 +566,18 @@ class WonderInk(QMainWindow): printer = QPrinter(QPrinter.PrinterMode.HighResolution) dialog = QPrintDialog(printer, self) + # No "Print to File". + # + # Qt offers it in every print dialog, and choosing it enables an + # output path with a "..." button beside it — which is a file + # chooser, in a room whose whole design is that there is no file + # chooser. Nothing in a classroom wants a worksheet written to a + # PDF nobody can then open; the room has a printer, and that is + # what this dialog is for. + dialog.setOption( + QAbstractPrintDialog.PrintDialogOption.PrintToFile, False + ) + if dialog.exec() != QPrintDialog.DialogCode.Accepted: return @@ -431,46 +633,6 @@ class WonderInk(QMainWindow): self._show_tab_bar() -def downloads_to(): - """Somewhere a download can land without asking a teacher anything.""" - target = Path( - os.environ.get("XDG_DOWNLOAD_DIR") or Path.home() / "Downloads" - ) - - try: - target.mkdir(parents=True, exist_ok=True) - except OSError: - target = Path.home() - - return target - - -def on_download(download): - """Accept it, put it somewhere sensible, and say so on screen. - - Wonder Ink hands out leader guides and colouring pages, so a download - that silently does nothing is a lesson a teacher cannot print. There - is no downloads bar to click, so the notification is the feedback. - """ - target = downloads_to() - download.setDownloadDirectory(str(target)) - download.accept() - - name = download.downloadFileName() - - def finished(): - state = download.state() - - if state == QWebEngineDownloadRequest.DownloadState.DownloadCompleted: - notify("Saved", f"{name} is in {target}") - elif state == QWebEngineDownloadRequest.DownloadState.DownloadCancelled: - pass - else: - notify("Download failed", name) - - download.isFinishedChanged.connect(finished) - - def notify(summary, body): import subprocess @@ -550,7 +712,6 @@ def main(): QWebEngineProfile.PersistentCookiesPolicy.NoPersistentCookies ) profile.setHttpUserAgent(plain_user_agent(profile)) - profile.downloadRequested.connect(on_download) # A lesson is not allowed to put a desktop notification on a classroom # television. The ordinary browser is told the same thing by policy. profile.setNotificationPresenter(lambda notification: None) @@ -586,11 +747,35 @@ def main(): settings.setAttribute( QWebEngineSettings.WebAttribute.JavascriptCanOpenWindows, True ) + # A worksheet is rendered by the engine's own PDF viewer, which is + # why there is no second rendering library in this room. settings.setAttribute( QWebEngineSettings.WebAttribute.PdfViewerEnabled, True ) + settings.setAttribute( + QWebEngineSettings.WebAttribute.PluginsEnabled, True + ) + # THE TWO DOORS A LOCAL FILE COULD OPEN. + # + # Qt leaves LocalContentCanAccessFileUrls on by default, which means a + # local document may read other local documents. Nothing in this room + # needs that — a PDF cannot use it anyway — and now that the shell + # loads file: URLs at all, a default is not a good enough answer. + # Both are named here so that the answer is in the file rather than in + # a Qt release note. + settings.setAttribute( + QWebEngineSettings.WebAttribute.LocalContentCanAccessFileUrls, False + ) + settings.setAttribute( + QWebEngineSettings.WebAttribute.LocalContentCanAccessRemoteUrls, False + ) + + DOWNLOADS.mkdir(parents=True, exist_ok=True) window = WonderInk(profile) + # Connected to the window rather than to a loose function: a finished + # worksheet becomes a tab, and only a window can make one. + profile.downloadRequested.connect(window.on_download) window.open_tab(HOME_PAGE) # An ordinary size before the maximised one, so the window has # something sensible to be if it is ever asked to stop being diff --git a/tests/test-deployment-support.sh b/tests/test-deployment-support.sh index ff759a1..ccbd098 100755 --- a/tests/test-deployment-support.sh +++ b/tests/test-deployment-support.sh @@ -267,6 +267,72 @@ check("Wonder Ink is not told twice", check("the clock is sized by the bar, not by a number", 'autoFontAndSize", true' in layout and "fontSize" not in layout) +print() +print("8. a lesson's worksheets come back as tabs") +# The workflow itself is driven against a running session; these are the +# properties of it that can be asserted from the source, and each one is +# something whose absence was a bug at some point in the lab. +check("worksheets land in Wonder Ink's own directory", + 'DOWNLOADS = Path(' in wonderink and '"Wonder Ink"' in wonderink) +check("a finished PDF is opened as a tab", + "def open_worksheet" in wonderink and "def open_pdf" in wonderink) +check("and only after its contents are checked", + 'PDF_MAGIC = b"%PDF-"' in wonderink and "head != PDF_MAGIC" in wonderink) +# isFinishedChanged is never emitted for an interrupted download, which +# was measured: a worksheet that did not arrive said nothing at all. +check("terminal states are read from stateChanged, not isFinishedChanged", + "download.stateChanged.connect" in wonderink + and "isFinishedChanged.connect" not in wonderink) +check("an interrupted download says so", + "DownloadInterrupted" in wonderink and "interruptReasonString" in wonderink) +check("the path is made into a URL rather than concatenated", + "QUrl.fromLocalFile" in wonderink) + +print() +print("8b. and opening them does not open the machine") +check("a navigation guard decides every local file", + "def acceptNavigationRequest" in wonderink and "url.isLocalFile()" in wonderink) +check("which consults a list of paths the window itself opened", + "def may_open" in wonderink and "self._openable" in wonderink) +check("local documents may not read other local documents", + "LocalContentCanAccessFileUrls, False" in wonderink) +check("nor reach the network", + "LocalContentCanAccessRemoteUrls, False" in wonderink) +check("there is no file chooser behind the print dialog", + "PrintDialogOption.PrintToFile, False" in wonderink) +check("nothing downloaded is executed", + "subprocess" not in wonderink.split("def notify")[0] + and "QDesktopServices" not in wonderink + and "xdg-open" not in wonderink) +check("and no general file browser was added", + "QFileDialog" not in wonderink) + +print() +print("8c. Wonder Ink and the browser stay apart") +# The download handler belongs to the Wonder Ink window and is fed by the +# Wonder Ink profile. Nothing watches a directory, and nothing asks where +# a file came from by looking at its name. +check("downloads are handled through Wonder Ink's own profile", + "profile.downloadRequested.connect(window.on_download)" in wonderink) +check("no directory watcher", + "QFileSystemWatcher" not in wonderink and "inotify" not in wonderink) +check("the browser is not touched by any of it", + "wonderink" not in browser.lower().replace("wonder ink", "") + or "Wonder Ink is a separate application" in browser) +check("and Chrome's download policy is still unmentioned", + not any(k.lower().startswith("download") for k in chrome_policy)) + +print() +print("8d. the 0.2.1 behaviour it must not regress") +check("cookies are still session-only", + "NoPersistentCookies" in wonderink) +check("the Present-view autoplay fix is still there", + "PlaybackRequiresUserGesture, False" in wonderink) +check("fullscreen still returns to a maximised window", + "WindowFullScreen" in wonderink) +check("still no new-tab button or address bar", + "setCornerWidget" not in wonderink and "QLineEdit" not in wonderink) + print() print("7b. the disc player is decided before a session starts") layout_unit = (root / "systemd/layout/ekklesia-classroom-layout.service").read_text() diff --git a/tools/lab/converged-check b/tools/lab/converged-check index 5e49aa5..1e67ddc 100755 --- a/tools/lab/converged-check +++ b/tools/lab/converged-check @@ -316,6 +316,8 @@ check "and Chrome's old one is not reused" 0 \ # The Present-view fix: a lesson may start its own video, and only this # profile may. The browser's policy is not touched, which the next two # checks are about. +check "cookies are still session-only" 1 \ + "$(in_lab 'grep -c "NoPersistentCookies" /usr/local/bin/classroom-wonderink || true')" check "a lesson may start its own video" 1 \ "$(in_lab 'grep -c "PlaybackRequiresUserGesture, False" /usr/local/bin/classroom-wonderink || true')" check "and the browser's policy still says nothing about autoplay" 0 \ @@ -325,6 +327,39 @@ check "the browser is still Chrome" 1 \ check "and the browser is untouched by any of this" 0 \ "$(in_lab 'grep -ci "pyqt\|webengine" /usr/local/bin/classroom-browser || true')" +section "a lesson's worksheets come back as tabs" +check "worksheets have a directory of Wonder Ink's own" 1 \ + "$(in_lab 'grep -c "\"Wonder Ink\"$" /usr/local/bin/classroom-wonderink || true')" +check "a finished PDF becomes a tab" 1 \ + "$(in_lab 'grep -c "def open_worksheet" /usr/local/bin/classroom-wonderink || true')" +check "checked on its contents, not its name" 1 \ + "$(in_lab 'grep -c "head != PDF_MAGIC" /usr/local/bin/classroom-wonderink || true')" +# Measured: isFinishedChanged is never emitted for an interrupted +# download, so a worksheet that did not arrive said nothing at all. +check "terminal states come from stateChanged" 0 \ + "$(in_lab 'grep -c "isFinishedChanged.connect" /usr/local/bin/classroom-wonderink || true')" +check "which local files may be opened is a list, not a directory" 1 \ + "$(in_lab 'grep -c "def may_open" /usr/local/bin/classroom-wonderink || true')" +check "and a guard consults it on every navigation" 1 \ + "$(in_lab 'grep -c "def acceptNavigationRequest" /usr/local/bin/classroom-wonderink || true')" +check "a local document cannot read other local documents" 1 \ + "$(in_lab 'grep -c "LocalContentCanAccessFileUrls, False" /usr/local/bin/classroom-wonderink || true')" +check "no file chooser behind the print dialog" 1 \ + "$(in_lab 'grep -c "PrintDialogOption.PrintToFile, False" /usr/local/bin/classroom-wonderink || true')" +check "nothing downloaded is handed to the desktop to run" 0 \ + "$(in_lab 'grep -cE "QDesktopServices|xdg-open" /usr/local/bin/classroom-wonderink || true')" +check "and no file browser was added" 0 \ + "$(in_lab 'grep -c "QFileDialog" /usr/local/bin/classroom-wonderink || true')" +# The separation the whole split exists for: Wonder Ink handles its own +# downloads because they arrive through its own profile, not because +# anything inspects a filename or watches a directory. +check "downloads arrive through Wonder Ink's own profile" 1 \ + "$(in_lab 'grep -c "profile.downloadRequested.connect(window.on_download)" /usr/local/bin/classroom-wonderink || true')" +check "nothing watches a directory" 0 \ + "$(in_lab 'grep -cE "QFileSystemWatcher|inotify" /usr/local/bin/classroom-wonderink || true')" +check "and the browser still knows nothing about any of it" 0 \ + "$(in_lab 'grep -cE "pdf|download" /usr/local/bin/classroom-browser || true')" + section "Wonder Ink can be turned off per device" check "device.conf carries the setting" 1 \ "$(in_lab 'grep -c "^WONDERINK_ENABLED=" /etc/ekklesia-classroom/device.conf || true')" diff --git a/tools/lab/from-package b/tools/lab/from-package index 615d407..78f7cb5 100755 --- a/tools/lab/from-package +++ b/tools/lab/from-package @@ -57,3 +57,14 @@ check "audio policy" "$(docker exec -u classroom -e XDG_RUNTIME_DIR=/run/user/10 # fresh install and a migrated one are supposed to be indistinguishable # from a teacher's chair, so they are asked the same questions. "$ROOT/tools/lab/converged-check" ${CHECK_ARGS:-} +converged=$? + +# And the one part of Wonder Ink that is a behaviour rather than a +# property: which local files it will open. Run against the command the +# package installed, not against the repository. +echo +echo "Wonder Ink, worksheets and local files" +"$ROOT/tools/lab/pdf-check" +pdf=$? + +exit $(( converged | pdf )) diff --git a/tools/lab/pdf-check b/tools/lab/pdf-check new file mode 100755 index 0000000..95f6887 --- /dev/null +++ b/tools/lab/pdf-check @@ -0,0 +1,139 @@ +#!/bin/bash +# Does Wonder Ink open the worksheets it should and refuse the files it +# should not? +# +# The download-to-tab workflow is driven by hand against a running +# session; this is the part of it that can be asserted rather than looked +# at — which local files the shell will load, and which it will not. +# +# It runs inside the lab, because the classroom's own PyQt6 is there and +# the development host has no Qt at all. It imports the *installed* +# command, so it is testing what the package put on the machine. +set -uo pipefail + +NAME="${LAB_NAME:-classroom-v2-lab}" + +docker exec -i \ + -e QT_QPA_PLATFORM=offscreen \ + -e QTWEBENGINE_CHROMIUM_FLAGS="--no-sandbox --disable-gpu" \ + -e XDG_RUNTIME_DIR=/tmp/pdfcheck-rt \ + -e HOME=/root \ + "$NAME" bash -c 'mkdir -p /tmp/pdfcheck-rt && chmod 700 /tmp/pdfcheck-rt && python3 -' <<'PY' +import importlib.util, sys, tempfile +from importlib.machinery import SourceFileLoader +from pathlib import Path + +loader = SourceFileLoader("wonderink", "/usr/local/bin/classroom-wonderink") +spec = importlib.util.spec_from_loader("wonderink", loader) +ink = importlib.util.module_from_spec(spec) +loader.exec_module(ink) + +from PyQt6.QtCore import QUrl +from PyQt6.QtWidgets import QApplication +from PyQt6.QtWebEngineCore import QWebEngineProfile, QWebEnginePage + +app = QApplication(sys.argv[:1]) +profile = QWebEngineProfile("pdfcheck", app) +window = ink.WonderInk(profile) + +passed = failed = 0 + +def check(label, condition, detail=""): + global passed, failed + if condition: + print(f" \033[32mPASS\033[0m {label}") + passed += 1 + else: + print(f" \033[31mFAIL\033[0m {label}" + (f" [{detail}]" if detail else "")) + failed += 1 + +work = Path(tempfile.mkdtemp()) +real = work / "worksheet.pdf" +real.write_bytes(b"%PDF-1.4\n% a worksheet\n") +spaces = work / "Leader Guide, Week 1 (Advent).pdf" +spaces.write_bytes(b"%PDF-1.4\n% another\n") +htmlish = work / "broken.pdf" +htmlish.write_bytes(b"404") +empty = work / "empty.pdf" +empty.write_bytes(b"") +notes = work / "notes.txt" +notes.write_text("bring scissors") + +print("1. which local files the window will open") +check("nothing at all, before anything is downloaded", + not window.may_open(Path("/etc/passwd"))) +check("not a file that merely exists", not window.may_open(real)) + +page = ink.LessonPage(profile, window) +check("and a page may not navigate to one", + page.acceptNavigationRequest( + QUrl.fromLocalFile("/etc/passwd"), + QWebEnginePage.NavigationType.NavigationTypeLinkClicked, True) is False) +check("nor to a worksheet that was never opened", + page.acceptNavigationRequest( + QUrl.fromLocalFile(str(real)), + QWebEnginePage.NavigationType.NavigationTypeLinkClicked, True) is False) + +print() +print("2. a worksheet the window itself opened") +window.open_pdf(real) +check("is openable afterwards", window.may_open(real)) +check("and only it", not window.may_open(Path("/etc/passwd"))) +check("a page may navigate to that one", + page.acceptNavigationRequest( + QUrl.fromLocalFile(str(real)), + QWebEnginePage.NavigationType.NavigationTypeLinkClicked, True) is True) +check("a symlink to somewhere else is not laundered into permission", + not window.may_open(Path("/etc/passwd"))) + +print() +print("3. which schemes are allowed at all") +for scheme, want in (("https://app.wonderink.org/", True), + ("http://example.org/", True), + ("about:blank", True), + ("mailto:someone@example.org", False), + ("ftp://example.org/x", False), + ("javascript:alert(1)", False)): + check(f"{scheme} -> {want}", + page.acceptNavigationRequest( + QUrl(scheme), + QWebEnginePage.NavigationType.NavigationTypeLinkClicked, True) is want) + +print() +print("4. which downloads become a worksheet tab") +before = window._tabs.count() +check("a real PDF does", window.open_worksheet(spaces, "application/pdf") is True) +check("and it made a tab", window._tabs.count() == before + 1) +check("a PDF by extension alone does", window.open_worksheet(spaces) is True) + +before = window._tabs.count() +check("HTML called .pdf does not open a blank tab", + window.open_worksheet(htmlish, "application/pdf") is True + and window._tabs.count() == before) +check("an empty file does not either", + window.open_worksheet(empty, "application/pdf") is True + and window._tabs.count() == before) +check("a file that is not there at all is handled", + window.open_worksheet(work / "missing.pdf", "application/pdf") is True + and window._tabs.count() == before) +check("plain text is not a worksheet and is left to the notification", + window.open_worksheet(notes, "text/plain") is False + and window._tabs.count() == before) +check("and a text file named .pdf is refused on its contents", + window.open_worksheet(htmlish) is True and window._tabs.count() == before) + +print() +print("5. names with spaces and punctuation survive the round trip") +window.open_pdf(spaces) +url = QUrl.fromLocalFile(str(spaces.resolve())) +# toString() is the display form and shows the spaces as spaces; the +# encoded form is what actually goes on the wire. +check("the URL percent-encodes rather than breaks", + "%20" in bytes(url.toEncoded()).decode() + and url.toLocalFile() == str(spaces.resolve())) +check("and the window will open exactly that path", window.may_open(spaces)) + +print() +print(f"{passed} passed, {failed} failed") +sys.exit(1 if failed else 0) +PY