Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a113b7bd62 |
@@ -1832,21 +1832,6 @@ is not it.** A `placeholder` is an accname fallback, so an
|
|||||||
Explore's search box — the audit's own `a11y.26` — as clean. A sweep
|
Explore's search box — the audit's own `a11y.26` — as clean. A sweep
|
||||||
for *empty* names cannot see a *weak* one.
|
for *empty* names cannot see a *weak* one.
|
||||||
|
|
||||||
**`title` is the same trap one rung lower, and it defeats the obvious
|
|
||||||
spec as well as the obvious sweep.** `queue-panel`'s Clear queue and
|
|
||||||
Add queue to playlist were named by `title` alone, so
|
|
||||||
`getByRole('button', { name: 'Clear queue' })` matched them **before**
|
|
||||||
the fix as well as after — a `getByRole` assertion, which is what
|
|
||||||
catches every other nameless control in this app, would have been
|
|
||||||
green on the broken build. `title` is the *last* fallback in the
|
|
||||||
accname order, so content put inside the button later silently
|
|
||||||
outranks it, and it is the one name a phone cannot show, having no
|
|
||||||
hover. The property is therefore asserted as *the name is not the
|
|
||||||
tooltip*: `queue-overlay.spec.ts` removes the `title` attributes and
|
|
||||||
asks again, which is 1 and 1 with `aria-label` and was measured at 0
|
|
||||||
and 0 without it. The `title`s stay, because on a desktop they are
|
|
||||||
also the tooltip for an icon-only control and that is a different job.
|
|
||||||
|
|
||||||
**The shell scrolls sideways and not down.** `body` is
|
**The shell scrolls sideways and not down.** `body` is
|
||||||
`overflow-x: auto; overflow-y: hidden`, and both halves are measured.
|
`overflow-x: auto; overflow-y: hidden`, and both halves are measured.
|
||||||
Vertically there is nothing to fix: the middle grid row is `1fr` and
|
Vertically there is nothing to fix: the middle grid row is `1fr` and
|
||||||
|
|||||||
@@ -63,12 +63,15 @@ func Parse(r io.Reader) ([]Chunk, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
data := make([]byte, size)
|
// Copied rather than allocated up front, as ID3Chunk does: the
|
||||||
if _, err := io.ReadFull(r, data); err != nil {
|
// size is four bytes off the file, so a truncated one is free to
|
||||||
|
// declare a chunk larger than the whole of itself.
|
||||||
|
var data bytes.Buffer
|
||||||
|
if _, err := io.CopyN(&data, r, int64(size)); err != nil {
|
||||||
return nil, fmt.Errorf("read chunk data for %q: %w", id, err)
|
return nil, fmt.Errorf("read chunk data for %q: %w", id, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
chunks = append(chunks, Chunk{ID: id, Data: data})
|
chunks = append(chunks, Chunk{ID: id, Data: data.Bytes()})
|
||||||
|
|
||||||
// Odd-length chunks have a padding byte. Lenient: if the
|
// Odd-length chunks have a padding byte. Lenient: if the
|
||||||
// read fails (e.g. EOF), just break rather than error.
|
// read fails (e.g. EOF), just break rather than error.
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"encoding/binary"
|
"encoding/binary"
|
||||||
"errors"
|
"errors"
|
||||||
|
"runtime"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"yellowjacket/backend/riff"
|
"yellowjacket/backend/riff"
|
||||||
@@ -209,3 +210,45 @@ func TestParse_ReadsEveryChunkInOrder(t *testing.T) {
|
|||||||
t.Errorf("odd chunk data: got %q, want %q", chunks[1].Data, "INFOodd")
|
t.Errorf("odd chunk data: got %q, want %q", chunks[1].Data, "INFOodd")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A chunk size is four bytes read off the file, so a truncated or
|
||||||
|
// malformed WAV is free to declare a chunk larger than the whole of
|
||||||
|
// itself. Parse must grow with what arrives rather than with what was
|
||||||
|
// claimed.
|
||||||
|
//
|
||||||
|
// This measures the allocation instead of the error because the error
|
||||||
|
// is the same either way: a build sizing its buffer from the header
|
||||||
|
// reports the truncation correctly, having asked the allocator for a
|
||||||
|
// gigabyte on the way. Deliberately not parallel — TotalAlloc is
|
||||||
|
// process-wide, and a test paused beside another one is measuring it
|
||||||
|
// too.
|
||||||
|
func TestParse_DoesNotAllocateWhatAChunkClaims(t *testing.T) {
|
||||||
|
// Large enough that a header-sized buffer is unmistakable, in a
|
||||||
|
// container of a few dozen bytes.
|
||||||
|
const declared = 1 << 30
|
||||||
|
|
||||||
|
var raw bytes.Buffer
|
||||||
|
|
||||||
|
raw.WriteString("RIFF")
|
||||||
|
_ = binary.Write(&raw, binary.LittleEndian, uint32(declared+12))
|
||||||
|
raw.WriteString("WAVE")
|
||||||
|
raw.WriteString("data")
|
||||||
|
_ = binary.Write(&raw, binary.LittleEndian, uint32(declared))
|
||||||
|
raw.WriteString("and then the file ends")
|
||||||
|
|
||||||
|
var before, after runtime.MemStats
|
||||||
|
|
||||||
|
runtime.GC()
|
||||||
|
runtime.ReadMemStats(&before)
|
||||||
|
|
||||||
|
if _, err := riff.Parse(bytes.NewReader(raw.Bytes())); err == nil {
|
||||||
|
t.Fatal("Parse: got nil error for a chunk larger than the file holding it")
|
||||||
|
}
|
||||||
|
|
||||||
|
runtime.ReadMemStats(&after)
|
||||||
|
|
||||||
|
if grew := after.TotalAlloc - before.TotalAlloc; grew > 1<<20 {
|
||||||
|
t.Errorf("Parse allocated %d bytes reading a %d-byte file whose chunk header claimed %d",
|
||||||
|
grew, raw.Len(), declared)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -157,62 +157,6 @@ test.describe('an overlaid queue says it is over the content', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
|
||||||
* #170 — the other two buttons in that same row.
|
|
||||||
*
|
|
||||||
* Clear queue and Add queue to playlist predate the close button and
|
|
||||||
* were named by a `title` attribute and nothing else. Unlike the
|
|
||||||
* sliders in `control-names.spec.ts`, that is not a *missing* name:
|
|
||||||
* `title` is the last fallback in the accname order, so
|
|
||||||
* `getByRole('button', { name: 'Clear queue' })` matched them before
|
|
||||||
* this fix as well as after it — measured, 1 and 1. A sweep for empty
|
|
||||||
* names cannot see a weak one, which is `a11y.26`'s complaint and the
|
|
||||||
* reason this file could have grown a green test that proved nothing.
|
|
||||||
*
|
|
||||||
* So the name is asserted twice, and the second assertion is the one
|
|
||||||
* that fails on the broken build. Taking the tooltip away and asking
|
|
||||||
* again is the property in words: **the name is not the tooltip**. It
|
|
||||||
* is what makes the button survive content being put inside it later,
|
|
||||||
* and it is the only one of the two a phone has — there is no hover on
|
|
||||||
* the surface #55 turned into a full screen. Measured on `main` before
|
|
||||||
* the fix: 0 and 0.
|
|
||||||
*
|
|
||||||
* Both buttons are disabled here, because the queue starts empty and
|
|
||||||
* naming is not enablement. A disabled button is still in the
|
|
||||||
* accessibility tree, which is exactly where the complaint was.
|
|
||||||
*/
|
|
||||||
test.describe('the queue header says what its actions do', () => {
|
|
||||||
const ACTIONS = ['Clear queue', 'Add queue to playlist'];
|
|
||||||
|
|
||||||
test('names both of the older actions', async ({ app }) => {
|
|
||||||
await openQueue(app);
|
|
||||||
|
|
||||||
for (const name of ACTIONS) {
|
|
||||||
await expect(
|
|
||||||
app.getByRole('button', { name, exact: true }),
|
|
||||||
).toHaveCount(1);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
test('and the names do not come from the tooltip', async ({ app }) => {
|
|
||||||
await openQueue(app);
|
|
||||||
|
|
||||||
await app.locator('#queue-panel').evaluate((el) => {
|
|
||||||
for (const button of el.shadowRoot!.querySelectorAll(
|
|
||||||
'.header-action-button',
|
|
||||||
)) {
|
|
||||||
button.removeAttribute('title');
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
for (const name of ACTIONS) {
|
|
||||||
await expect(
|
|
||||||
app.getByRole('button', { name, exact: true }),
|
|
||||||
).toHaveCount(1);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The inline panel is the mode that already worked, and the one every
|
* The inline panel is the mode that already worked, and the one every
|
||||||
* other queue spec is written against. It keeps its resize handle and
|
* other queue spec is written against. It keeps its resize handle and
|
||||||
|
|||||||
@@ -2201,25 +2201,11 @@ export class QueuePanel
|
|||||||
`
|
`
|
||||||
: nothing}
|
: nothing}
|
||||||
</div>
|
</div>
|
||||||
<!-- **Every action here is named by aria-label**, like
|
|
||||||
the close button #24 added beside them (#170). A
|
|
||||||
title alone *is* a name, which is why a sweep for
|
|
||||||
empty names reports these clean and why an
|
|
||||||
assertion by role and name is green either way --
|
|
||||||
but it is the weakest one: title is the last
|
|
||||||
fallback in the accname order, so any content put
|
|
||||||
inside the button later silently outranks it, and
|
|
||||||
a phone has no hover to show it as a tooltip.
|
|
||||||
|
|
||||||
The titles stay. On a desktop they are the tooltip
|
|
||||||
for an icon-only control, which is a different job
|
|
||||||
from naming it, and aria-label does not do it. -->
|
|
||||||
<div class="header-actions">
|
<div class="header-actions">
|
||||||
<button
|
<button
|
||||||
class="header-action-button"
|
class="header-action-button"
|
||||||
@click=${() => void this.handleClearQueue()}
|
@click=${() => void this.handleClearQueue()}
|
||||||
?disabled=${tracks.length === 0}
|
?disabled=${tracks.length === 0}
|
||||||
aria-label="Clear queue"
|
|
||||||
title="Clear queue"
|
title="Clear queue"
|
||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
@@ -2230,7 +2216,6 @@ export class QueuePanel
|
|||||||
class="header-action-button add-to-playlist-button"
|
class="header-action-button add-to-playlist-button"
|
||||||
@click=${this.handleAddToPlaylist}
|
@click=${this.handleAddToPlaylist}
|
||||||
?disabled=${tracks.length === 0}
|
?disabled=${tracks.length === 0}
|
||||||
aria-label="Add queue to playlist"
|
|
||||||
title="Add queue to playlist"
|
title="Add queue to playlist"
|
||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
|
|||||||
Reference in New Issue
Block a user