Prove the place marker's accessible name against the accessibility tree #200
Loading…
Reference in a new issue
No description provided.
Delete branch "marker-accessible-names"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Task 262 (#84). ADR-0098 amended in place, ADR-0071 corrected in place; no new number.
MarkerAccessibleNameJourneyTestsreads the marker's own node out of Chromium's accessibility tree over CDP (Accessibility.getPartialAXTree) and asserts it is not ignored, is abutton, is named "{place name}, {group name}", and took that name fromaria-labelrather than a weaker source.The ticket's premise turned out to be half right.
alton adivIconis indeed dead — removed from the place markers and from the current-location dot, with a comment at each call site saying why it cannot come back. But the label does reach assistive technology, and has since ADR-0071, through the explicitaria-labelon the icon element; ADR-0098 was wrong about the mechanism, not about the outcome. Both records now say so, and ADR-0071 carries a pointer to the full account.Watched to fail three ways, all three discriminating:
altalone,titleandaria-labelremoved — the accessible name is empty. The direct proof the original claim was false.Home.BuildMarker's label reddens on the name.aria-labelreddens on the source,"aria-label"vs"title". The first version of this test asserted the name alone and stayed green through that mutation: the marker would have degraded silently to being named by its tooltip.Two judgements a reviewer may want to overturn. The test reloads the page before reading, because
Homemarks the whole mapinertwhile any overlay is open (ADR-0086) and an inert subtree is withheld from the accessibility tree outright — the alternative was waiting on theinertattribute itself, which couples the test to that mechanism. Andtitlestill duplicates the name as the node'sdescription, which some screen readers will announce twice; left alone because ADR-0098 values that hover tooltip for sighted mouse users.Suite: 2210 passing, 4 skipped across the solution; E2E 57 passing.
Verdict: changes needed
The premise check comes out in your favour:
aria-labelis set on every activatable marker's icon from theaddhandler, Leaflet nulls_iconon remove so a fresh element — and a freshadd— is guaranteed on every path that produces one (initial plot,setMarkersrebuild, a marker breaking back out of a cluster), andHomewiresOnMarkerActivated, soactivatableis true there. Nothing in the suite read the removedalt. No behavioural defect is being closed as a documentation fix.What needs changing is the argument, not the test. Both the ADR and the test's class doc justify reading the AX tree on the grounds that a DOM assertion on
alt"would have passed throughout" — it would have gone red the day the pin became adiv, and is precisely the assertion that would have caught this. The CDP read is still the right level for a different reason: it proves the outcome regardless of which attribute supplies it. Inline on both.ADR-0071 still carries the falsified sentence (and the
<img>premise behind thearia-label) with nothing pointing at the correction, andmap.js's own comments send readers there. Inline.On the three you flagged:
name.sourcescomes back in the same AXNode and names the winning attribute, so pinningaria-labelcosts no DOM reading — see inline. Suggestion, not a blocker; recording the non-discriminating mutation was the right call either way.inertwould couple the test to ADR-0086's mechanism.titleduplicating the name as description: leave it. Dropping it costs the tooltip and the fallback for the verbosity of one repeated announcement in some readers.@ -207,0 +217,4 @@'IMG')`; the Decision above replaced Leaflet's default `<img>` pin with an `L.divIcon` so a pin couldbe recoloured, and from that moment the `alt` `map.js` kept passing was dropped silently. Nothingannounced it: no error, no warning, and the attribute the option would have produced simply neverappeared. ADR-0071's own equivalent sentence was true when written, for an `<img>` pin, and wasADR-0071 lines 149–153 still say
Labelis threaded to Leaflet'salt, and still justify the explicitaria-labelby "an<img>whose role is overridden" — both now false of the code, andmap.js's comments point readers at that record. Give ADR-0071 the same one-lineCorrected:pointer this one got.@ -207,0 +225,4 @@in headless Chromium and reads the marker's own node out of the browser's accessibility tree over CDP(`Accessibility.getPartialAXTree`), asserting it is not ignored, is a `button`, and is named"{place name}, {group name}". Reading the *computed* node is the whole point: every DOM-levelassertion available — that `alt` is set, that `aria-label` is set — would have passed against theSame misstatement as the test's class doc: an assertion that
altis set would have failed against the broken code, not passed. The narrower true version is that no DOM assertion tells you what the platform computes, and thearia-labelone passes either way.@ -0,0 +13,4 @@/// an <c>alt</c> option, ADR-0098 recorded the label as "read aloud by a screen reader" partly on the/// strength of it, and Leaflet applies <c>alt</c> only to an <c>IMG</c> — on the <c>divIcon</c> pin/// every marker has been drawn with since that same record it was dropped silently. A test asserting/// that <c>alt</c> was set would have passed throughout and proved nothing about what a screen readeraltwas never applied to thediv, sogetAttribute("alt")would have gone red the day the pin stopped being an<img>— that assertion would have caught this, not missed it. Reword to the claim that holds: no DOM assertion tells you what the platform computes as the name, and thearia-labelone passes whether or not the name ever reaches assistive technology.@ -0,0 +46,4 @@// still unreachable — a distinct failure from having no name, and worth telling apart.marker.Ignored.ShouldBeFalse();marker.Role.ShouldBe("button");marker.Name.ShouldBe($"{placeName}, {groupName}");Optional:
getPartialAXTreereturnsname.sourcesalongsidename. Asserting the winning source'sattributeisaria-labelwould redden the mutation you recorded as non-discriminating, still reading the computed tree rather than the DOM.a63abc5dcabbfaa453c5bbfaa453c58ed9f98eb3Verdict: changes needed
Reviewed at
8ed9f98. One thing left, and it is the same species as the one just fixed, four lines below the correction that fixed it: ADR-0071 still says a null label falls back to "Leaflet's own default,\"Marker\"", andsrc/PlaceMark.WebUI/Maps/MapMarker.cs'sLabelparam doc says it verbatim. That fallback was thealtdefault. A picker marker is never activatable, so it gets noaria-label;titleresolves to"", which Leaflet does not set; and there is noalt. A null-labelled marker now has no accessible name at all — the right answer for a non-interactive pin, and not what either record says.MapMarker.csis the copy a developer actually reads. Inline on the ADR line.Everything else verified at this head:
name.sourcesis genuinely stronger, not differently worded. Chromium lists every candidate in accname order and marks the loserssuperseded, so "first source that carries a value and is not superseded" is DevTools' own rule rather than an ordering guess of the test's; with thearia-labelgone,titlebecomes the first unsuperseded source carrying a value, which is the"aria-label"vs"title"red you saw. If Chromium ever changes that shape the test fails loudly on the wrong source name rather than passing silently, so the fragility falls the right way.altassertion would indeed have gone red the day the pin became adiv, and the three things named as beyond the DOM's reach — exposure, computed name and role, which attribute wins — are each carried by an assertion (Ignored,Role/Name,NameSource). The tree-level justification does exactly the work claimed and no more.altremoval changes nothing observable. Against mergedmain:currentLocationIconis anL.divIcon, so thataltwas equally dead;CurrentLocationJourneyTestsreads class andtabindex, neveralt; ADR-0167 never claimed a name for the dot, so nothing else needs correcting there. No name is the right answer forinteractive: false+keyboard: false.@ -154,1 +169,3 @@place's own name; `PlaceForm`'s one picker marker passes nothing, falling back to Leaflet's own default,Leaflet's own `alt` and `title` marker options and to the explicit `aria-label` above (`alt` nolonger — see the correction above). `Home` passes the place's own name; `PlaceForm`'s one pickermarker passes nothing, falling back to Leaflet's own default,This clause is the third casualty of the same 2026-08-11 change, and survived the rewrite: "Leaflet's own default,
\"Marker\"" is thealtdefault, and nothing applies it now.PlaceForm's picker marker is not activatable, soaddKeyboardActivationnever runs and noaria-labelis set;titleresolves to"", which Leaflet skips; there is noalt. That marker has no accessible name at all — correct for a non-interactive pin, but say that instead.MapMarker.cs'sLabelparam doc carries the same sentence and is the copy a developer reads.Verdict: mergeable
One line inline on ADR-0071's front matter, not blocking.
Verified at
b8b8357:titleparagraph is still true, and needed the subject change it got:toLeafletMarkerspassestitlefor every marker regardless ofactivatable, so "not only when a marker is activatable" is the live contrast. The empty-label case it does not cover is stated in the correction immediately above it.a3bf693, byte-identical tomain's), not against the prose:Marker's defaults aretitle: "",alt: "Marker";_initIcondoest.title && (i.title = t.title), so an emptytitleis skipped, and"IMG" === i.tagName && (i.alt = t.alt || ""), so that"Marker"default only ever reached an<img>.LeafletMapcreates an activation reference only whenOnMarkerActivated.HasDelegate, and onlyHomewires it — so the picker marker genuinely gets noaria-label, andgeneric/no name is the expected computed answer.name.sources, the three tree assertions and the dot'saltremoval are unchanged from8ed9f98.The rate-limiting flakes are worth their own ticket, not a fold into 263. Same family — a test racing wall-clock under load — but a different mechanism and a much cheaper fix. Both classes set
RateLimiting:Auth:Windowto 2 seconds with a permit limit of 2, then make three real HTTP requests that must all land inside one window.AuthenticationRateLimiterTestsdoes a deliberately expensive password hash on every one of them (ADR-0035's own point), andOidcRateLimitingTestspays the stub provider's discovery on the first. If the window rolls between the first request and the third, the third is permitted and the429assertion fails — exactly the observed shape, and exactly why it is those two classes and nothing else. The limiter is not measuring anything wrongly; the test is given a two-second budget for work whose duration it does not control.The remedy is not 263's injectable clock. The window is already per-class configuration, and
PostLogin_AfterTheWindowElapses_TheSameAddressIsPermittedAgainis the only test that has to observe expiry — it currently forces every other test in its class to share its short window. Give the enforcement tests a long window (tens of seconds; they never wait it out) and leave the expiry test its own short one. No timeout is raised anywhere, which keeps it inside 263's own constraint.@ -1,6 +1,11 @@# ADR-0071: Extend the Leaflet interop boundary again, so clicking a marker selects its place- **Status:** Accepted (partly superseded by 0101, 0131)- **Corrected:** 2026-08-20, working Vikunja task 262 — two sentences of this record's ownUndercounts its own body now: two correction blockquotes covering three claims (the two
altclauses, the threading sentence, the"Marker"fallback), while this says "two sentences" and "the correction ... below", singular.