main fix: pause the clock while an Activity-opened thread must stay unread - #548
Conversation
Main run 37034458144 (539bb13) failed in WebKit: thread-unread.spec.mjs line 253 found "Thread root 1" already read. Its one read publication held thread-activity for that root, which only a thread panel on that root can write. The test opens that thread from the Activity popover, then closes it, and expects it to stay unread. page.clock.install() fakes timers, but the clock keeps flowing. When the close took longer than the 300 ms reading dwell, the open panel earned it and read the thread. The test comment assumed the clock was stopped. Pause the clock from just before that open until focus returns to the sidebar, then resume. This is the pattern message-navigation.spec.mjs already uses. It is the only spec that releases reading focus under an installed clock without pausing it. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
No material findings. The pause is narrowly placed before reading focus is released and ends after focus returns to the sidebar. It prevents incidental dwell during the open/close handoff; subsequent real reading, independent unread markers, durable publication, and reload assertions remain intact. No product code or assertions changed.
Validation: inspected the complete diff, focus helper, 300 ms reading/cancellation path, and existing clock precedent. CI run 37038534088 passed at this head; logs confirm all five tests in the affected spec passed in both Chromium and WebKit. The original WebKit failure matches the claimed unread assertion. I did not rerun tests locally or independently reproduce the author's forced-delay experiment. This supports this bounded fix, not a claim that all timing flakes are eliminated.
Reviewed head 24b7e1c55d5bd52531bbb82d55e040b3e912e49f against base 539bb13578d7c8c5a4d42002112857d66e20e19b.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
🤖
Summary
539bb135(run 37034458144).thread-unread.spec.mjs:52expected "Thread root 1" to still show "Observed unread replies", but it was already read.page.clock.install()fakes timers but lets time keep flowing, so a slow close let the open thread pass 300 ms and get read. The test comment assumed the clock was stopped.Details
thread-activity:<Thread root 1>. Only an open thread panel for that root can write that entry, and the only time this test opens that root is the Activity popover step.message-navigation.spec.mjsalready uses the samepauseAt/resumepattern for the same reason. I searched every browser spec that installs the clock: this is the only one that releases reading focus under an installed clock without pausing it. The other release in this file (same-thread sidebar activity…) makes no unread assertion after the open.thread-activityentry. With this fix, the same forced wait passes 8/8.--repeat-each 10on Chromium + WebKit passed 20/20. Fullthread-unread.spec.mjson both engines passed 10/10. Biome passed.