Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ac57083
test(support): extract narrow_frame_fits? from open_in_narrow_frame (…
plainprogrammer Nov 12, 2026
8d0da39
test(support): narrow-frame helper compares with clientWidth, not inn…
plainprogrammer Nov 12, 2026
4740f48
docs(support): document the narrow-frame helper's params, platform sc…
plainprogrammer Nov 12, 2026
7770869
test(support): page_fits_sideways?, the one measure of sideways overf…
plainprogrammer Nov 12, 2026
8086c13
test(system): seven specs measure sideways overflow with the shared h…
plainprogrammer Nov 12, 2026
bbc9aef
test(system): seven specs' single overflow checks call page_fits_side…
plainprogrammer Nov 12, 2026
5ec2aed
test(system): overflow measures inside JS results compare with client…
plainprogrammer Nov 12, 2026
82234de
test(support): open_in_viewport_frame opens a frame as wide as a phon…
plainprogrammer Nov 12, 2026
1d4d4e2
test(system): deck page overflow checks use clientWidth and a true 36…
plainprogrammer Nov 12, 2026
22260b2
test(system): shape panel overflow example uses clientWidth and a tru…
plainprogrammer Nov 12, 2026
24d6d0d
test(system): add-to-deck edge checks compare with clientWidth (#318)
plainprogrammer Nov 12, 2026
ab3e353
test(system): footer gutter and scanner width checks measure the view…
plainprogrammer Nov 12, 2026
edb136b
docs(support): review fixes for open_in_viewport_frame's contract, de…
plainprogrammer Nov 12, 2026
f4147e7
docs(lessons): sweep a measurement trap by swapping the measure and r…
plainprogrammer Nov 12, 2026
c95b761
Merge origin/main into 079-narrow-frame-honest-overflow (spec 078, #336)
plainprogrammer Nov 12, 2026
ff8ddbf
test(system): shape panel phone frames reuse open_in_viewport_frame (…
plainprogrammer Nov 12, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/memory/headless-firefox-narrow-frame.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ metadata:

Headless Firefox (Selenium, system specs and screenshot scripts) clamps the window to at least 500px wide. A `driven_by … screen_size: [390, 844]` driver actually renders at 500px, so phone-width assertions silently test the wrong layout.

- **Specs:** use `spec/support/narrow_frame.rb` (`open_in_narrow_frame(path, width:, height:, ready:)` returns `[innerWidth, no_horizontal_overflow]`, and `within_narrow_frame { … }`) with the normal driver. Media queries and container queries evaluate against the iframe's own width. Prove each narrow assertion by breaking the CSS once. Touch-target and coarse-pointer rules still can't be tested this way. The window must be taller than the frame: links below the window's bottom edge can't be clicked, and the click silently does nothing (seed 12061, 2026-10-31).
- **Specs:** use `spec/support/narrow_frame.rb` (`open_in_narrow_frame(path, width:, height:, ready:)` returns `[innerWidth, no_horizontal_overflow]`, where "no overflow" compares `scrollWidth` with `clientWidth` since #318, and `narrow_frame_fits?` re-measures from the top page after a change inside the frame; `page_fits_sideways?` is the one overflow measure for the page the driver is in, top page or inside `within_narrow_frame`, so never write `scrollWidth <= innerWidth` again; `open_in_viewport_frame(path, viewport:, ready:)` opens a frame whose viewport is the width asked for and returns the viewport it got, which the example asserts; `within_narrow_frame { … }`) with the normal driver. Media queries and container queries evaluate against the iframe's own width. Prove each narrow assertion by breaking the CSS once. Touch-target and coarse-pointer rules still can't be tested this way. The window must be taller than the frame: links below the window's bottom edge can't be clicked, and the click silently does nothing (seed 12061, 2026-10-31).
- **Screenshots:** inject a fixed-width iframe on a same-origin page, grow the frame to its content height, resize the window taller than the frame, then take an element screenshot of the frame. Without the taller window only the visible part is captured.
- Separately named drivers (`options: { name: :firefox_dark }`) are still needed for different Firefox prefs, e.g. the dark theme, because Capybara reuses sessions by driver name.

Expand All @@ -20,4 +20,4 @@ Related: [[pr-screenshots-workflow]].
- Measure related boxes in one script call. Two calls raced on the scanner page, which grows while the camera starts.
- **Keyboard-only selects (spec 041, 2026-11-06):** under WebDriver in headless Firefox, arrow keys don't change a closed `<select>`'s value (its `selectedIndex` stays 0 whether or not the list opens). A keyboard-only example chooses by type-ahead instead: focus the select and `send_keys` the option's first letter.
- **Scrollbars are pinned to classic (#271, 2026-11-10):** on a Mac, Firefox follows "Show scroll bars: Automatic" and draws overlay scrollbars (no width) when no mouse is connected, which made a 360px frame 15px wider than CI's and broke a wrap check locally only. `spec/support/system.rb` now sets `ui.useOverlayScrollbars = 0`. A driver registered under another name (the dark-theme ones) needs the same pref if it opens a narrow frame. See `docs/lessons/2026-11-10-a-local-only-failure-was-the-macs-scrollbar-setting.md`.
- **A 360px frame is not a 360px phone (spec 069, 2026-11-11):** the frame keeps the forced classic scrollbar (15px on the Mac, about 12px on CI) and the body has an 8px margin, so a 360px frame's page container is 297px (360 − 15 − 16 body − 32 padding) and a deck-shape panel's interior 263px; a real 360px viewport gives a 312px container and a 278px interior. Model a phone as a **375px frame** (a 360px viewport). The `@container page` breakpoints (640px, 992px) are on the container (frame − 15 − 16 − page padding): a 640px frame is a 545px container and still shows the fold; the aside needs a frame of 735px or more. Write the chain in the spec; see `docs/lessons/2026-11-11-a-phone-width-criterion-needs-a-measured-chain.md` and #318 (the helper misses overflow a scrollbar hides). The pinned classic scrollbar is 15px on the Mac but about 12px on the Linux CI runner (spec 069's first CI run, interior 281px vs 278px): size a phone frame from the measured `innerWidth - documentElement.clientWidth`, `360 + scrollbar`, not 375.
- **A 360px frame is not a 360px phone (spec 069, 2026-11-11):** the frame keeps the forced classic scrollbar (15px on the Mac, about 12px on CI) and the body has an 8px margin, so a 360px frame's page container is 297px (360 − 15 − 16 body − 32 padding) and a deck-shape panel's interior 263px; a real 360px viewport gives a 312px container and a 278px interior. Model a phone with `open_in_viewport_frame` (a 360px viewport; a 375px frame is only right where the scrollbar is 15px). The `@container page` breakpoints (640px, 992px) are on the container (frame − 15 − 16 − page padding): a 640px frame is a 545px container and still shows the fold; the aside needs a frame of 735px or more. Write the chain in the spec; see `docs/lessons/2026-11-11-a-phone-width-criterion-needs-a-measured-chain.md` and #318 (the helper used to miss overflow a scrollbar hides; it compares with `clientWidth` now). The pinned classic scrollbar is 15px on the Mac but about 12px on the Linux CI runner (spec 069's first CI run, interior 281px vs 278px): `open_in_viewport_frame` sizes the frame from the measured scrollbar (`360 + innerWidth - clientWidth`), not 375. The zone table of the deck page's worst-case row (60-char name, 1,234 proxies) spills 7 to 10px past a 360px frame's 345 to 348px layout but not a true 360px viewport (spec 074, #318).
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
date: 2026-11-12
spec: none (direct fix, #318)
tags: [system-specs, firefox, layout, narrow-frame, measurement]
---

# Lesson: sweep a measurement trap by swapping the measure and running, not by grep

## Context

#318: the narrow-frame helper compared `scrollWidth` with `innerWidth`, which includes the frame's classic scrollbar (15px on the Mac, about 12px on CI), so a page overflowing sideways by up to that much passed. Spec 072's review had named the trap on 2026-11-11 (`docs/lessons/2026-11-11-two-measurement-traps-the-classic-scrollbar-and-closed-details-menus.md`). The helper fix was four lines.

## What happened

- The helper's change alone left all 452 examples that use it green. The same trap was written 25 more times as inline `scrollWidth <= window.innerWidth` checks in 17 other specs, nine of them as a local `fits?` method each. A grep found those.
- The first review round found them, and a second grep still missed two variants: `footer_spec` compared the footer text's right edge with `width - gutter`, where `width` is the frame's width and so includes the scrollbar, and `scanner_sitting_spec` compared `scrollWidth` with the literal 360. Neither contains `innerWidth`. The second round's reviewer found them by reading.
- Swapping the measure to `clientWidth` and running was what found real findings: 5 examples failed, all at a "360px" narrow frame of the deck page. The cause was not a bug: a 360px frame with a 15px scrollbar lays a page out in 345px (348px on CI), and the zone table's worst-case row spills 7 to 10px past that, but not past a true 360px viewport (spec 074 recorded the spill and its limit). The examples had been passing because the scrollbar hid it, and "360px" meant two layouts in one file.
- The honest fix was to say which 360px each example means: `open_in_viewport_frame` opens a frame as the viewport plus the measured scrollbar, and the two overflow examples use it. The helper is the one measure now (`page_fits_sideways?`), so the next check has nothing to copy.

## What to do next time

- To remove a measurement trap, put the correct measure in one helper first, then swap every use and run the files. Grep finds the copies that look like the trap; the run finds the ones that behave like it. A comparison against a frame's width, a viewport literal, or a width variable that includes a scrollbar is the same trap without its name.
- When a stricter measure turns an example red, ask what viewport the name promises ("360px" is a phone's viewport, not a 360px frame) before touching the page.
- Probe before asking for a scope ruling: a temporary `sed` over the files, one run and a `git checkout` took under four minutes for 17 files and let the question to the coordinator name the 5 failing examples and their cause.

## Signals to watch for

- `scrollWidth` or an element's `right` compared with `innerWidth`, a frame `width` parameter, or a number that is a frame's width.
- A local `fits?` or `frame_fits?` defined in a spec: the measure belongs in `spec/support/narrow_frame.rb`.
- An example named for a phone width that opens `open_in_narrow_frame(…, width: 360)` and checks overflow or an edge.
59 changes: 54 additions & 5 deletions spec/support/narrow_frame.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
# the page in a fixed-width same-origin iframe (id "narrow") on top of the current page.
module NarrowFrame
# Loads path in a width × height frame, waits for the ready selector inside it and returns
# [the frame's innerWidth, whether its page fits without horizontal scrolling].
# [the frame's innerWidth, whether its page fits without horizontal scrolling (see narrow_frame_fits?)].
#
# @param path [String] the page to load, on the current origin
# @param width [Integer] the frame's width in px (its innerWidth, scrollbar included)
# @param ready [String] a CSS selector that must be present in the frame before it is measured
# @param height [Integer] the frame's height in px; shorter than the page, the frame draws a vertical scrollbar
# @return [Array(Integer, Boolean)] the frame's innerWidth and whether its page fits
def open_in_narrow_frame(path, width:, ready:, height: 800)
page.execute_script(<<~JS, path, width, height)
document.getElementById("narrow")?.remove()
Expand All @@ -13,10 +19,53 @@ def open_in_narrow_frame(path, width:, ready:, height: 800)
document.body.append(frame)
JS
within_narrow_frame { find(ready) }
page.evaluate_script(<<~JS)
(() => { const view = document.getElementById("narrow").contentWindow
return [ view.innerWidth, view.document.documentElement.scrollWidth <= view.innerWidth ] })()
JS
[ page.evaluate_script("document.getElementById('narrow').contentWindow.innerWidth"), narrow_frame_fits? ]
end

# Loads path in a frame whose viewport (its documentElement.clientWidth) is viewport px wide, the way a phone of that width is:
# open_in_narrow_frame's width includes the frame's classic scrollbar, so a "360px" frame there lays a page out in 345px or 348px.
# The frame is the viewport plus the scrollbar it draws, measured (about 15px on a Mac, 12px on the Linux runner) and reopened
# once if the first guess was wrong. A page shorter than the frame has no scrollbar and gets a frame exactly viewport px wide.
#
# It does not raise: when the reopened page gains or loses its scrollbar the viewport it got differs, so the caller asserts the
# width it returns.
#
# @param path [String] the page to load, on the current origin
# @param viewport [Integer] the width the page is laid out in, in px
# @param ready [String] a CSS selector that must be present in the frame before it is measured
# @param height [Integer] the frame's height in px
# @return [Integer] the viewport the frame got, for the example to assert
def open_in_viewport_frame(path, viewport:, ready:, height: 800)
width = viewport + 15
2.times do
open_in_narrow_frame(path, width:, ready:, height:)
scrollbar = within_narrow_frame { page.evaluate_script("window.innerWidth - document.documentElement.clientWidth") }
break if viewport + scrollbar == width

width = viewport + scrollbar
end
within_narrow_frame { page.evaluate_script("document.documentElement.clientWidth") }
end

# Whether the page the driver is in (the top page, or the frame's inside within_narrow_frame) fits without horizontal
# scrolling, measured now. The one measure of sideways overflow, so a check never has to write it again (#318).
#
# The page's scrollWidth is compared with the documentElement's clientWidth, the width the layout has, and not with
# innerWidth: innerWidth includes a vertical scrollbar (classic in system specs, #271, about 15px on a Mac and 12px on
# CI), which the page has whenever it is taller than its window or frame, so a page scrolling sideways by up to that
# much would pass.
#
# @return [Boolean]
def page_fits_sideways?
page.evaluate_script("document.documentElement.scrollWidth <= document.documentElement.clientWidth")
end

# Whether the page in the narrow frame fits without horizontal scrolling, measured now (see page_fits_sideways?).
# Call it from the top page, outside within_narrow_frame, which it enters itself.
#
# @return [Boolean]
def narrow_frame_fits?
within_narrow_frame { page_fits_sideways? }
end

def within_narrow_frame(&)
Expand Down
4 changes: 1 addition & 3 deletions spec/system/bulk_move_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
let(:account) { user.account }
let!(:blue) { create(:location, account:, name: "Blue binder") }

def frame_fits? = page.evaluate_script("(() => { const view = document.getElementById('narrow').contentWindow; return view.document.documentElement.scrollWidth <= view.innerWidth })()")

# Tab until the element with this id has focus, as a keyboard does.
def tab_to(id)
20.times do
Expand Down Expand Up @@ -79,6 +77,6 @@ def tab_to(id)
expect(page).to have_css("h1", text: "Move 9,999 items").and have_css(".c-appbar__back")
expect(page).to have_select("To", options: [ "Choose…", "No location", "Blue binder" ])
end
expect(frame_fits?).to be(true)
expect(narrow_frame_fits?).to be(true)
end
end
8 changes: 3 additions & 5 deletions spec/system/card_page_decks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

def set_theme(name) = page.execute_script("document.documentElement.dataset.theme = #{name.to_json}")

def fits? = page.evaluate_script("document.documentElement.scrollWidth <= window.innerWidth")

describe "the stats strip (AC-5.1)" do
before do
older = create(:mtg_printing, entry: create(:catalog_entry, identity: card.identity, released_on: Date.new(2020, 1, 1))).entry
Expand All @@ -36,7 +34,7 @@ def fits? = page.evaluate_script("document.documentElement.scrollWidth <= window
clipped: parts.some((el) => el.scrollWidth > el.clientWidth + 1) } })()
JS
expect(shown).to eq("texts" => %w[24,690 12,345 12,345], "oneRow" => true, "clipped" => false)
expect(fits?).to be(true)
expect(page_fits_sideways?).to be(true)
end
end
end
Expand Down Expand Up @@ -67,7 +65,7 @@ def section_width = page.evaluate_script('document.querySelector("#in-decks .c-c
expect(shown_headers).to eq([ "Deck", "Owned", "Proxy" ])
expect(page).to have_css("#in-decks tbody tr:first-child .c-table__sub", text: "Modern · Main", visible: :visible)
expect(find("#in-decks td:first-child a", match: :first).native.rect.height).to be > 20 # the 60-character name wraps
expect(fits?).to be(true)
expect(page_fits_sideways?).to be(true)
end
end
end
Expand All @@ -81,7 +79,7 @@ def section_width = page.evaluate_script('document.querySelector("#in-decks .c-c
expect(shown_headers).to eq([ "Deck", "Zone", "Owned", "Proxy" ])
expect(page).to have_css("#in-decks tbody tr:first-child span.is-muted", text: "· Modern", visible: :visible)
expect(page).to have_no_css("#in-decks .c-table__sub", visible: :visible)
expect(fits?).to be(true)
expect(page_fits_sideways?).to be(true)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/system/card_page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def head_row_fits
return { oneLine: Math.max(...controls.map((c) => c.height)) >= box.height - 1, tall: controls.every((c) => c.height >= 44),
inside: controls.every((c) => c.left >= box.left - 0.5 && c.right <= box.right + 0.5),
unclipped: primary.scrollWidth <= primary.clientWidth + 1,
overflow: document.documentElement.scrollWidth > window.innerWidth } })()
overflow: document.documentElement.scrollWidth > document.documentElement.clientWidth } })()
JS
end

Expand Down
4 changes: 2 additions & 2 deletions spec/system/collection_import_preview_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def version_three_file(entry)

expect(page).to have_text("Its location “Nowhere in the file” isn't one of the file's locations.")
expect(page).to have_text("in Blue binder with a rather long name for a phone")
expect(page.evaluate_script("document.documentElement.scrollWidth <= window.innerWidth")).to be(true)
expect(page_fits_sideways?).to be(true)
end

# Spec 062 AC-7.1, AC-7.5: the collector's own export, decks and a copy in a zone included, previews with the Decks section; a
Expand Down Expand Up @@ -81,6 +81,6 @@ def preview_file(data)

expect(page).to have_css("#format_changed_decks li", text: "#{long_name}:")
expect(page).to have_css("#removed_decks li", text: "Elves")
expect(page.evaluate_script("document.documentElement.scrollWidth <= window.innerWidth")).to be(true)
expect(page_fits_sideways?).to be(true)
end
end
10 changes: 4 additions & 6 deletions spec/system/deck_additions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
RSpec.describe "Add to deck", type: :system do
def set_theme(name) = page.execute_script("document.documentElement.dataset.theme = #{name.to_json}") # no colour is read here, so no pause (testing.md: no sleep)

def fits? = page.evaluate_script("document.documentElement.scrollWidth <= window.innerWidth")

# The drawn height of every control of the page, by kind. `html` is pinned to a scrollbar in both pointer states, so a
# taller page can't narrow the layout by a scrollbar's width and move what wraps (spec 072's lesson).
def control_heights
Expand Down Expand Up @@ -39,13 +37,13 @@ def control_heights
open_in_narrow_frame(new_catalog_entry_deck_addition_path(wide_entry), width:, ready: "main")
within_narrow_frame do
set_theme(theme)
expect(fits?).to be(true)
expect(page_fits_sideways?).to be(true)
overflow = page.evaluate_script(<<~JS)
(() => { const form = document.querySelector(".c-form").getBoundingClientRect()
const select = document.querySelector("#deck_zone").getBoundingClientRect()
const text = document.createRange(); text.selectNodeContents(document.querySelector(".c-pick .c-check"))
return { select: select.width <= form.width, text: text.getBoundingClientRect().right <= document.querySelector(".c-pick").getBoundingClientRect().right + 0.5,
title: document.querySelector(".c-pagehead__title").getBoundingClientRect().right <= window.innerWidth } })()
title: document.querySelector(".c-pagehead__title").getBoundingClientRect().right <= document.documentElement.clientWidth } })()
JS
expect(overflow).to eq("select" => true, "text" => true, "title" => true)
end
Expand Down Expand Up @@ -73,8 +71,8 @@ def refusal_sentence_overflow
page.evaluate_script(<<~JS)
(() => { const sentence = document.querySelector(".c-field__error"), text = document.createRange() // the drawn text, not the box
text.selectNodeContents(sentence)
return { page: document.documentElement.scrollWidth <= window.innerWidth,
edge: text.getBoundingClientRect().right <= window.innerWidth, scroll: sentence.scrollWidth <= sentence.clientWidth } })()
return { page: document.documentElement.scrollWidth <= document.documentElement.clientWidth,
edge: text.getBoundingClientRect().right <= document.documentElement.clientWidth, scroll: sentence.scrollWidth <= sentence.clientWidth } })()
JS
end

Expand Down
Loading
Loading