Compare commits

...
Author SHA1 Message Date
Dmitry Verkhoturov d534247c5a Pin the public path #2203 fixed, which shipped without a test
The bundler used to bake a fixed public path into every entry, so an
instance mounted under a prefix, which manuals/subdomain documents,
asked the domain root for its provider icons and got nothing. #2197
derived the path from the URL the bundle was loaded from instead, and
covered it with nothing: publicPath appears only in webpack.config.js
and neither suite touched asset paths.

The check is on the assignment webpack emits for its runtime public
path, a string literal when the path is fixed and an expression when it
is derived. Asset filenames are bare in both builds, so the obvious
assertion, looking for a rooted "/web/name.svg" string, finds nothing
either way and proves nothing. I wrote that one first and it passed
against a deliberately broken build.

Every emitted bundle is checked rather than the entry the bug was
reported against: it put fifteen icons in remark.mjs and one in
last-comments.mjs, so a case reading a single entry would have gone
green with half of it still live.

Verified by rebuilding the frontend with the public path baked back in:
the assertion fails on that build and passes on master's.
2026-08-23 21:22:42 +01:00
Dmitry Verkhoturov 75e1b69342 Judge the iframe reveal by the mark, not by when the read lands
Three cases separate a reveal that came from the inited message from one that
came from the five second fallback, and two of them did it against a clock this
process holds. That is a bet on how fast an engine is, and both bets lose on a
loaded machine: TestIframe_StaysHiddenUntilTheDocumentReportsInited polled the
DOM after a loop bounded 500ms below the fallback, and an evaluate round trip
outlasts that margin, so a fallback firing exactly on time reads as an early
reveal; TestIframe_IsRevealedByTheInitedMessage capped the message path at 3s,
and webkit has reported inited 2.6s after creation here.

All three now judge the recorded reveal against one cutoff, half a second under
the fallback. A timer cannot fire early, so below it the reveal can only be the
message and above it only the fallback, and the mark carries the moment itself,
so a slow read cannot move it. The hidden case drops its polled visibility
assertion for the same mark, which is what the observer records in the first
place.
2026-08-23 21:22:42 +01:00
2 changed files with 56 additions and 21 deletions
+19 -21
View File
@@ -96,9 +96,12 @@ const (
// be satisfied by the fallback alone and says nothing about the message path. bound the
// message-path assertions well under it
revealTimeout = 5 * time.Second
// generous enough for a cold navigation on a loaded runner, and still well under the
// fallback, which is the point of the assertion
messageRevealBudget = 3 * time.Second
// the line the three reveal cases are judged against, measured from the element's creation
// and not from anything this process can time. a timer cannot fire early, so below this the
// reveal can only have come from the message, and above it only from the fallback. a budget
// chosen for how long an engine takes instead would be a bet on the slowest one: webkit has
// reported inited 2.6s after creation on a loaded machine
revealCutoff = revealTimeout - 500*time.Millisecond
)
// openWithBlockedIframeDoc loads the demo page with the widget document aborted, so the
@@ -242,21 +245,19 @@ func TestIframe_StaysHiddenUntilTheDocumentReportsInited(t *testing.T) {
forEachEngine(t, func(t *testing.T, page playwright.Page) {
openWithBlockedIframeDoc(t, page)
// unconditional: the loop below is bounded by the frame's own age, and on a slow
// enough load that bound can already be spent, which would leave the test asserting
// nothing at all about visibility
require.Equal(t, "hidden", iframeVisibility(t, page))
// then hold it for almost the whole fallback window. stopping halfway would only prove
// the fallback is not shorter than that, and a widget that revealed on anything other
// than `inited` would still pass. measured from the element's creation, since that is
// when the fallback it must not have used starts counting
for iframeAge(t, page) < revealTimeout-500*time.Millisecond {
require.Equal(t, "hidden", iframeVisibility(t, page))
// hold for almost the whole fallback window. stopping halfway would only prove the
// fallback is not shorter than that, and a widget that revealed on anything other than
// `inited` would still pass
for iframeAge(t, page) < revealCutoff {
time.Sleep(100 * time.Millisecond)
}
_, revealed := revealDelay(t, page)
assert.False(t, revealed, "the frame was revealed before its document reported inited")
// the mark and not what is visible when the read lands: the two are separated by the
// margin above, and an evaluate round trip on a loaded engine outlasts it, which reads
// a fallback that fired exactly on time as an early reveal
delay, revealed := revealDelay(t, page)
assert.False(t, revealed && delay < revealCutoff,
"the frame was revealed %v after it was created, before its document reported inited", delay)
})
}
@@ -290,7 +291,7 @@ func TestIframe_IsRevealedByTheInitedMessage(t *testing.T) {
// whole 5s window without the widget being at fault
delay, ok := revealDelay(t, page)
require.True(t, ok, "the frame reported no reveal at all")
assert.Less(t, delay, messageRevealBudget,
assert.Less(t, delay, revealCutoff,
"the reveal was slow enough to have come from the fallback and not the message")
waitVisible(t, page.Locator("#remark42 iframe"))
})
@@ -310,12 +311,9 @@ func TestIframe_IsRevealedByTheTimeoutWhenInitedNeverArrives(t *testing.T) {
// and not before it: without a lower bound, shortening the fallback to a value that
// defeats its purpose would still pass. against the frame's own clock, so that a slow
// navigation cannot be mistaken for the timer having run
// close to the fallback and not three quarters of it: measured in the page there is
// no navigation to make room for, and a wider floor tolerates a fallback shortened
// enough to defeat its purpose
delay, ok := revealDelay(t, page)
require.True(t, ok, "the frame reported no reveal at all")
assert.Greater(t, delay, revealTimeout-500*time.Millisecond,
assert.Greater(t, delay, revealCutoff,
"the reveal came too early to have been the fallback timer")
})
}
+37
View File
@@ -11,6 +11,7 @@ import (
"net/http"
"os"
"os/exec"
"regexp"
"slices"
"strings"
"sync"
@@ -264,3 +265,39 @@ func pauseForWebLimit() {
}
lastWebGet = time.Now()
}
// TestWeb_NoBundleHardcodesTheWebRoot pins the fix for #2203, which shipped with no test at any
// level. The bundler used to bake a fixed public path into every entry, so an instance mounted
// under a prefix, which manuals/subdomain documents, asked the domain root for its provider icons
// and got nothing. The path is derived from the URL the bundle was loaded from now, which is
// correct for both arrangements.
//
// What separates the two builds is the assignment webpack emits for its runtime public path: a
// literal when the path is fixed, and a computed value when it is derived. Asset filenames appear
// bare either way, so a case looking for a rooted "/web/name.svg" string finds nothing in either
// build and proves nothing; this asserts the assignment instead.
//
// Every emitted bundle is checked rather than the obvious one: the original defect put fifteen
// icons in remark.mjs and one in last-comments.mjs, so a case reading a single entry would have
// gone green with half of it still live.
func TestWeb_NoBundleHardcodesTheWebRoot(t *testing.T) {
names := emittedBundles(t)
require.Contains(t, names, "remark.mjs", "listing is not the served web root: %v", names)
require.Contains(t, names, "last-comments.mjs",
"the entry carrying the second half of #2203 is missing from the listing: %v", names)
// webpack writes its public path to the `p` property of the runtime object. A baked-in path
// is a string literal there; a derived one is an expression
baked := regexp.MustCompile("\\.p\\s*=\\s*[\"'`]/web/[\"'`]")
for _, name := range names {
t.Run(name, func(t *testing.T) {
body := getWeb(t, "/web/"+name).body
require.NotEmpty(t, body, "%s serves nothing, so this asserts nothing", name)
assert.NotRegexp(t, baked, body,
"%s bakes the public path in rather than deriving it, so an instance mounted under "+
"a prefix fetches its assets from the domain root", name)
})
}
}