Conversation
both for lyrics and metadata
feat(docs): New API
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a hosting/respond helper and migrates handlers across features to HTMX-aware respond.* helpers, moves routes off the ChangesRespond helpers and feature refactoring
Sequence DiagramsequenceDiagram
participant Client
participant StreamHandler
participant StreamingService
participant Filesystem
Client->>StreamHandler: GET /stream?path=<encoded>
StreamHandler->>StreamingService: Stream(decodedPath)
StreamingService->>Filesystem: resolve & validate path
Filesystem-->>StreamingService: resolved path + mime
StreamingService-->>StreamHandler: path + mime
alt Accept: application/json
StreamHandler->>Client: { "type", "url" } JSON
else
StreamHandler->>Client: audio bytes (stream)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels: ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
views/importing/queue_header.html (1)
17-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
typeattribute to button elements.HTML5 requires the
typeattribute on<button>elements. Without it, buttons default totype="submit", which can trigger unintended form submissions. Addtype="button"to all action buttons that should not submit forms.🔧 Proposed fix
<button + type="button" hx-get="/import/queue/items"Apply the same fix to all other button elements in this file (lines 27, 43, 57, 69).
Also applies to: 27-36, 43-55, 57-68, 69-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/importing/queue_header.html` around lines 17 - 26, Add an explicit type attribute to each plain action button so they don't default to type="submit": locate the button element with attributes hx-get="/import/queue/items" and title="Refresh" (inner text "Refresh") and add type="button"; repeat the same change for the other action buttons in this template (any <button> elements used for actions/refreshes) so all non-form-submit buttons include type="button".src/features/downloading/handlers.go (2)
206-227:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFire the job-list refresh trigger after starting downloads.
Unlike the other job-start handlers in this PR, these endpoints return
respond.ToastJobwithout settingHX-Trigger. The download succeeds, but the jobs UI will stay stale until its next refresh after Lines 227, 249, 271, 298, and 331.Also applies to: 230-250, 253-272, 275-299, 302-332
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/downloading/handlers.go` around lines 206 - 227, After successfully starting a job in DownloadTrack (after h.service.DownloadTrack returns jobID), set the HX-Trigger header on the Fiber context so the jobs UI is refreshed (for example set c.Set("HX-Trigger", `{"job-list-refresh": null}`) or equivalent) before returning respond.ToastJob; apply the same fix to the other job-start handlers in this file that call h.service.* and return respond.ToastJob so each response includes the HX-Trigger for "job-list-refresh".
334-369:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't swap raw JSON into the album-tracks target on failure.
This handler now renders
downloading/album_trackson success, but Lines 340 and 347 still return JSON errors. In the HTMX flow, that will insert raw JSON into the panel instead of showing a toast or an HTML error state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/downloading/handlers.go` around lines 334 - 369, The handler GetAlbumTracks currently returns JSON error responses when albumID is missing and when h.service.GetAlbumTracks fails, which will insert raw JSON into the HTMX panel; replace those two c.Status(...).JSON(...) returns (the albumID check and the err != nil branch after h.service.GetAlbumTracks) with calls to respond.Partial (or the app's HTML error partial) so the HTMX flow gets an HTML fragment—invoke respond.Partial with the same HTTP status and pass a simple map like {"Error": errMsg} (preserve context like the downloader if useful) so the "downloading/album_tracks" panel shows a proper HTML error instead of raw JSON.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/api.md`:
- Around line 25-206: The API docs still list legacy routes using the `/ui`
prefix (e.g. GET `/ui`, `/ui/dashboard`, `/ui/library`, `/ui/analyze/lyrics`,
`/ui/jobs/count`) while the updated templates and handlers now use non-`/ui`
paths (e.g. `/dashboard`, `/library`, `/jobs/count`, `/analyze/lyrics`); update
the route tables by replacing `/ui/...` entries with the new top-level routes,
ensure the examples in the
Config/Library/Tag/Importing/Jobs/Download/Lyrics/Playlists/Reorganize/Metrics
sections match the new routes and HTTP responses shown in the diff, and add a
short note/redirect entry where appropriate for any legacy `/ui` → new path
redirects so consumers aren’t misled.
- Around line 3-5: The statement claiming "Every endpoint supports dual content
negotiation via the `HX-Request` header" is too broad; update the wording to
restrict the claim to only those endpoints that actually perform content
negotiation (e.g., endpoints that return Resource representations and those that
explicitly support HTMX). Replace "Every endpoint" with a qualified phrase like
"Endpoints that return Resource responses" or "Most endpoints that support both
HTML and API clients" and add a short note that some endpoints are always JSON
or HTMX-only and instead use Accept header negotiation for Resource responses;
reference the `HX-Request` header, "Resource responses", and "Accept"
negotiation in the edit so readers know which cases use which mechanism.
In `@src/features/lyrics/handlers.go`:
- Around line 69-90: GetLyricsProviders currently returns JSON on success for
non-HTMX clients but returns plain text/HTML on errors; update error branches
(e.g., the empty track ID check, the metadataService.GetTrackFileTags error
path, and any other branches inside GetLyricsProviders and similar handlers
between lines 94-117) to return structured JSON error responses when the request
is not HTMX (check c.Get("HX-Request") != "true") instead of
c.Status(...).SendString or respond.Partial; use c.Status(...).JSON(...) with a
consistent error shape (message and optional code) so both success and failure
for non-HTMX callers are JSON, while still using respond.Partial for HTMX flows
like respond.Partial(c, "tag/lyrics_buttons", ...).
In `@src/features/metadata/handlers.go`:
- Around line 605-627: The GetMetadataProviders handler returns plain strings
for errors when HX-Request != "true", which breaks JSON consumers; update
GetMetadataProviders so that when c.Get("HX-Request") != "true" all responses
(including error cases) use JSON payloads with consistent shape (e.g.,
{"error":"message"} or {"error":"message","details":...}); specifically change
the two plain-string error returns (the empty trackID check and the
GetTrackFileTags error path) to return c.Status(...).JSON(...) and keep the HTMX
branch using respond.Partial for template rendering; reference the
GetMetadataProviders function and the calls to
h.service.GetEnabledMetadataProviders and h.service.GetTrackFileTags to locate
where to replace SendString with JSON responses.
In `@src/features/playlists/handlers.go`:
- Around line 296-299: The response sets the M3U content type inconsistently:
respond.Resource is called with "audio/x-mpegurl" but the inner callback
overrides it to "text/plain"; update the callback in handlers.go (the anonymous
function passed to respond.Resource in the playlist export handler) to not
override the Content-Type or to set it to "audio/x-mpegurl" instead of
"text/plain" (leave the Content-Disposition filename logic intact) so clients
receive the correct MIME type; ensure the change is made where
c.Set("Content-Type", ...) and c.Set("Content-Disposition", ...) are invoked.
In `@src/features/ui/routes.go`:
- Around line 9-12: Routes were refactored to remove the /ui prefix but other
code and docs still point to /ui paths; update all references so they match the
new routes. Replace occurrences of '/ui/library/tag/edit/'+track.ID and any
other '/ui/...' links in src/features/lyrics/lyrics_job.go (manual_fix HTML),
change documented endpoints in docs/api.md to their new paths (remove /ui prefix
and update redirect notes), and update examples in FEATURE_SPEC_KIT.md
(hx-get/redirect examples) to use the new root-based routes (e.g.,
/library/tag/edit/:id, /dashboard, /analyze, /dashboard/quick-actions) so code
and docs match handler.RenderDashboard, handler.RenderAnalyzeSection and
handler.GetQuickActionsCard routing.
In `@views/playlists/playlist.html`:
- Around line 4-5: The HX action buttons are missing an explicit type which
causes them to default to submit; update both button elements (the one with
hx-get="/playlists" and the other "Browse Library" HX button) to include
type="button" so they won't trigger form submission and to satisfy the HTML lint
rule; locate the <button> elements that have hx-get/hx-post attributes and add
the type="button" attribute to each.
In `@views/sections/library.html`:
- Around line 5-6: The refresh button element identified by attributes
hx-get="/library" and hx-target="`#contenido`" lacks a type attribute and may
default to type="submit" inside forms; update that <button> (the element with
hx-get="/library" hx-swap="outerHTML" hx-target="`#contenido`") to explicitly
include type="button" to prevent accidental form submissions.
---
Outside diff comments:
In `@src/features/downloading/handlers.go`:
- Around line 206-227: After successfully starting a job in DownloadTrack (after
h.service.DownloadTrack returns jobID), set the HX-Trigger header on the Fiber
context so the jobs UI is refreshed (for example set c.Set("HX-Trigger",
`{"job-list-refresh": null}`) or equivalent) before returning respond.ToastJob;
apply the same fix to the other job-start handlers in this file that call
h.service.* and return respond.ToastJob so each response includes the HX-Trigger
for "job-list-refresh".
- Around line 334-369: The handler GetAlbumTracks currently returns JSON error
responses when albumID is missing and when h.service.GetAlbumTracks fails, which
will insert raw JSON into the HTMX panel; replace those two
c.Status(...).JSON(...) returns (the albumID check and the err != nil branch
after h.service.GetAlbumTracks) with calls to respond.Partial (or the app's HTML
error partial) so the HTMX flow gets an HTML fragment—invoke respond.Partial
with the same HTTP status and pass a simple map like {"Error": errMsg} (preserve
context like the downloader if useful) so the "downloading/album_tracks" panel
shows a proper HTML error instead of raw JSON.
In `@views/importing/queue_header.html`:
- Around line 17-26: Add an explicit type attribute to each plain action button
so they don't default to type="submit": locate the button element with
attributes hx-get="/import/queue/items" and title="Refresh" (inner text
"Refresh") and add type="button"; repeat the same change for the other action
buttons in this template (any <button> elements used for actions/refreshes) so
all non-form-submit buttons include type="button".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6ee921c6-8fcd-4e5e-b8b4-62584a006db0
📒 Files selected for processing (54)
docs/api.mdsrc/features/config/handlers.gosrc/features/config/routes.gosrc/features/downloading/handlers.gosrc/features/downloading/routes.gosrc/features/hosting/respond/respond.gosrc/features/importing/handlers.gosrc/features/importing/routes.gosrc/features/jobs/handlers.gosrc/features/jobs/routes.gosrc/features/library/handlers.gosrc/features/library/routes.gosrc/features/lyrics/handlers.gosrc/features/lyrics/routes.gosrc/features/metadata/handlers.gosrc/features/metadata/routes.gosrc/features/metrics/handlers.gosrc/features/metrics/routes.gosrc/features/playlists/handlers.gosrc/features/playlists/routes.gosrc/features/reorganize/handlers.gosrc/features/reorganize/routes.gosrc/features/ui/handlers.gosrc/features/ui/routes.goviews/cards/quick_actions.htmlviews/config/config_form.htmlviews/importing/queue_header.htmlviews/library/track_overview_panel.htmlviews/library/unified_search_list.htmlviews/lyrics/queue_header.htmlviews/lyrics/queue_items.htmlviews/metrics/overview.htmlviews/partials/job_status.htmlviews/partials/main.htmlviews/partials/navbar.htmlviews/partials/sidebar.htmlviews/playlists/add_to_playlist_modal.htmlviews/playlists/playlist.htmlviews/sections/analyze.htmlviews/sections/analyze_files.htmlviews/sections/analyze_lyrics.htmlviews/sections/analyze_metadata.htmlviews/sections/dashboard.htmlviews/sections/download.htmlviews/sections/import.htmlviews/sections/jobs.htmlviews/sections/library.htmlviews/sections/playlists.htmlviews/sections/settings.htmlviews/sections/tag.htmlviews/tag/edit_form.htmlviews/tag/lyrics_buttons.htmlviews/tag/metadata_buttons.htmlviews/tag/search_results_modal.html
Comment on lines
+605
to
627
| // GetMetadataProviders returns metadata provider buttons for HTMX or provider list as JSON. | ||
| func (h *Handler) GetMetadataProviders(c *fiber.Ctx) error { | ||
| trackID := c.Params("trackId") | ||
| if trackID == "" { | ||
| return c.Status(fiber.StatusBadRequest).SendString("Track ID is required") | ||
| } | ||
|
|
||
| // Get track data for button context | ||
| providers := h.service.GetEnabledMetadataProviders() | ||
|
|
||
| if c.Get("HX-Request") != "true" { | ||
| return c.JSON(providers) | ||
| } | ||
|
|
||
| track, err := h.service.GetTrackFileTags(c.Context(), trackID) | ||
| if err != nil { | ||
| slog.Error("Failed to get track for buttons", "error", err, "trackId", trackID) | ||
| slog.Error("Failed to get track for metadata providers", "error", err, "trackId", trackID) | ||
| return c.Status(fiber.StatusInternalServerError).SendString("Failed to load track data") | ||
| } | ||
|
|
||
| return c.Render("tag/metadata_buttons", fiber.Map{ | ||
| return respond.Partial(c, "tag/metadata_buttons", fiber.Map{ | ||
| "Track": track, | ||
| "EnabledProviders": h.service.GetEnabledMetadataProviders(), | ||
| "EnabledProviders": providers, | ||
| }) |
There was a problem hiding this comment.
Return JSON errors from GetMetadataProviders for non-HTMX requests.
The success path is JSON when HX-Request != true, but Lines 609 and 621 still use plain-string responses. API callers cannot rely on a stable payload shape once this endpoint fails.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/metadata/handlers.go` around lines 605 - 627, The
GetMetadataProviders handler returns plain strings for errors when HX-Request !=
"true", which breaks JSON consumers; update GetMetadataProviders so that when
c.Get("HX-Request") != "true" all responses (including error cases) use JSON
payloads with consistent shape (e.g., {"error":"message"} or
{"error":"message","details":...}); specifically change the two plain-string
error returns (the empty trackID check and the GetTrackFileTags error path) to
return c.Status(...).JSON(...) and keep the HTMX branch using respond.Partial
for template rendering; reference the GetMetadataProviders function and the
calls to h.service.GetEnabledMetadataProviders and h.service.GetTrackFileTags to
locate where to replace SendString with JSON responses.
Comment on lines
+296
to
+299
| return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error { | ||
| c.Set("Content-Type", "text/plain") | ||
| c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename)) | ||
| return c.SendString(m3uContent) |
There was a problem hiding this comment.
Keep the exported M3U content type consistent.
respond.Resource declares audio/x-mpegurl, but the callback overrides it to text/plain. This can cause clients to treat playlist exports incorrectly.
Proposed fix
return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error {
- c.Set("Content-Type", "text/plain")
+ c.Set("Content-Type", "audio/x-mpegurl")
c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename))
return c.SendString(m3uContent)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error { | |
| c.Set("Content-Type", "text/plain") | |
| c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename)) | |
| return c.SendString(m3uContent) | |
| return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error { | |
| c.Set("Content-Type", "audio/x-mpegurl") | |
| c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename)) | |
| return c.SendString(m3uContent) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/playlists/handlers.go` around lines 296 - 299, The response sets
the M3U content type inconsistently: respond.Resource is called with
"audio/x-mpegurl" but the inner callback overrides it to "text/plain"; update
the callback in handlers.go (the anonymous function passed to respond.Resource
in the playlist export handler) to not override the Content-Type or to set it to
"audio/x-mpegurl" instead of "text/plain" (leave the Content-Disposition
filename logic intact) so clients receive the correct MIME type; ensure the
change is made where c.Set("Content-Type", ...) and c.Set("Content-Disposition",
...) are invoked.
contre95
changed the title
contre95
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
FEATURE_SPEC_KIT.md (2)
87-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unresolved merge-conflict markers.
Line 87 through Line 93 still contains conflict markers, which leaves the spec in a broken state and can mislead contributors.
Suggested fix
-<<<<<<< Updated upstream -**Error Response Pattern**: -- HTMX errors: `src/features/downloading/handlers.go:43-46` -- API errors: `src/features/downloading/handlers.go:47-50` - -======= ->>>>>>> Stashed changes +**Error Response Pattern**: +- HTMX errors: `src/features/downloading/handlers.go:43-46` +- API errors: `src/features/downloading/handlers.go:47-50`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FEATURE_SPEC_KIT.md` around lines 87 - 93, Remove the leftover merge-conflict markers surrounding the "Error Response Pattern" section so the spec is valid: delete the "<<<<<<< Updated upstream", "=======" and ">>>>>>> Stashed changes" markers and reconcile the final content under the "Error Response Pattern" heading (keep the intended bullet list referencing HTMX errors `src/features/downloading/handlers.go:43-46` and API errors `src/features/downloading/handlers.go:47-50`, or merge them into one clear block). Ensure only the resolved, clean text remains and no conflict markers are left anywhere in the "Error Response Pattern" section.
99-100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate legacy
/uirouting guidance to the new root-level pattern.Line 99 still documents
/ui-grouped UI routes, but this refactor moves UI navigation to root-level grouped paths. Keeping this line as-is will cause new features to be scaffolded with stale routes.Suggested fix
-- UI routes under `/ui` group for HTMX partials +- UI routes grouped at root-level feature paths (e.g., `/dashboard`, `/library`, `/analyze/...`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FEATURE_SPEC_KIT.md` around lines 99 - 100, Replace the legacy guidance that says "UI routes under `/ui` group for HTMX partials" with the new root-level routing pattern so scaffolds generate root-level grouped UI paths (e.g., use "UI routes at root-level grouped paths for HTMX partials" instead of `/ui`), and keep the accompanying note that "Route registration should only pass the feature's own service to handlers" unchanged; update any example text in FEATURE_SPEC_KIT.md that references `/ui` to instead reference the root-level grouping convention so new features scaffold correct routes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/lyrics/lyrics_job.go`:
- Line 144: The log builds a manual_fix URL by concatenating track.ID directly,
which can break for reserved characters; update the code that calls
job.Logger.Error (the statement referencing "manual_fix") to path-escape
track.ID using url.PathEscape(track.ID) before embedding it in the href so the
generated link is safe and valid (keep the rest of the log fields intact and use
the escaped value for the manual_fix string).
---
Outside diff comments:
In `@FEATURE_SPEC_KIT.md`:
- Around line 87-93: Remove the leftover merge-conflict markers surrounding the
"Error Response Pattern" section so the spec is valid: delete the "<<<<<<<
Updated upstream", "=======" and ">>>>>>> Stashed changes" markers and reconcile
the final content under the "Error Response Pattern" heading (keep the intended
bullet list referencing HTMX errors `src/features/downloading/handlers.go:43-46`
and API errors `src/features/downloading/handlers.go:47-50`, or merge them into
one clear block). Ensure only the resolved, clean text remains and no conflict
markers are left anywhere in the "Error Response Pattern" section.
- Around line 99-100: Replace the legacy guidance that says "UI routes under
`/ui` group for HTMX partials" with the new root-level routing pattern so
scaffolds generate root-level grouped UI paths (e.g., use "UI routes at
root-level grouped paths for HTMX partials" instead of `/ui`), and keep the
accompanying note that "Route registration should only pass the feature's own
service to handlers" unchanged; update any example text in FEATURE_SPEC_KIT.md
that references `/ui` to instead reference the root-level grouping convention so
new features scaffold correct routes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ee05c793-6f50-44ea-b24d-2757412fb618
📒 Files selected for processing (3)
FEATURE_SPEC_KIT.mddocs/api.mdsrc/features/lyrics/lyrics_job.go
| result, err := t.service.AddLyrics(ctx, track.ID, provider, overrideNoQueue) | ||
| if err != nil { | ||
| job.Logger.Error("Failed to add lyrics for track", "trackID", track.ID, "title", track.Title, "provider", provider, "error", err.Error(), "manual_fix", "<a href='/ui/library/tag/edit/"+track.ID+"' target='_blank'>track</a>") | ||
| job.Logger.Error("Failed to add lyrics for track", "trackID", track.ID, "title", track.Title, "provider", provider, "error", err.Error(), "manual_fix", "<a href='/tag/"+track.ID+"' target='_blank'>track</a>") |
There was a problem hiding this comment.
Path-escape track.ID before embedding it in manual_fix URL.
Line 144 builds the URL with raw concatenation. If track.ID contains reserved characters, the link can break. Use url.PathEscape(track.ID) when composing the path.
Suggested fix
import (
"context"
"fmt"
"log/slog"
+ "net/url"
@@
- job.Logger.Error("Failed to add lyrics for track", "trackID", track.ID, "title", track.Title, "provider", provider, "error", err.Error(), "manual_fix", "<a href='/tag/"+track.ID+"' target='_blank'>track</a>")
+ job.Logger.Error("Failed to add lyrics for track", "trackID", track.ID, "title", track.Title, "provider", provider, "error", err.Error(), "manual_fix", "<a href='/tag/"+url.PathEscape(track.ID)+"' target='_blank'>track</a>")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| job.Logger.Error("Failed to add lyrics for track", "trackID", track.ID, "title", track.Title, "provider", provider, "error", err.Error(), "manual_fix", "<a href='/tag/"+track.ID+"' target='_blank'>track</a>") | |
| import ( | |
| "context" | |
| "fmt" | |
| "log/slog" | |
| "net/url" | |
| ) | |
| // ... other code ... | |
| job.Logger.Error("Failed to add lyrics for track", "trackID", track.ID, "title", track.Title, "provider", provider, "error", err.Error(), "manual_fix", "<a href='/tag/"+url.PathEscape(track.ID)+"' target='_blank'>track</a>") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/lyrics/lyrics_job.go` at line 144, The log builds a manual_fix
URL by concatenating track.ID directly, which can break for reserved characters;
update the code that calls job.Logger.Error (the statement referencing
"manual_fix") to path-escape track.ID using url.PathEscape(track.ID) before
embedding it in the href so the generated link is safe and valid (keep the rest
of the log fields intact and use the escaped value for the manual_fix string).
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.go (1)
137-150:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStart the Fiber server only once, after wiring signal handling.
This block starts the same server twice. The first
server.Start()blocks and bypasses the intended graceful-shutdown path; the later goroutine start is redundant and can fail with address-in-use errors.Suggested fix
streamingService := streaming.NewService(cfgManager) server := hosting.NewServer(cfgManager, importingService, libraryService, playlistsService, downloadingService, jobService, tagService, lyricsService, metricsService, reorganizeService, streamingService) -slog.Info("Starting server", "port", cfgManager.Get().Server.Port) -if err := server.Start(); err != nil { - slog.Error("server stopped: %v", "error", err) -} quit := make(chan os.Signal, 1) signal.Notify(quit, os.Interrupt) serverErr := make(chan error, 1) go func() { slog.Info("Starting server", "port", cfgManager.Get().Server.Port) if err := server.Start(); err != nil { serverErr <- err } }()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.go` around lines 137 - 150, The code currently calls server.Start() twice causing a blocking start and potential address-in-use errors; remove the first blocking call to server.Start() (the top-level call before signal handling) and instead set up signal handling (quit channel and signal.Notify) and serverErr channel first, then start the Fiber server exactly once inside the goroutine (where server.Start() is already invoked), using the existing serverErr and quit channels to handle shutdown and errors; ensure you still log the startup via slog.Info and perform graceful shutdown (server.Shutdown or equivalent) when quit or an error is received.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/library/service.go`:
- Around line 397-403: The current error handling maps any error from
s.library.GetTrack(ctx, trackID) to "track not found", masking real storage/DB
errors; change it so that if err != nil you return the wrapped err (e.g.,
fmt.Errorf("failed to get track: %w", err)) and only return the "track not
found" error when track == nil; update the block around the GetTrack call
(referencing s.library.GetTrack, trackID and track) to propagate real lookup
errors and reserve the not-found message solely for a nil track.
In `@src/features/streaming/handlers.go`:
- Around line 22-31: The handler currently calls h.service.Stream even when the
"path" query is empty, causing a 404 instead of a 400; update the code that
reads rawPath/path to validate that rawPath (or the decoded path) is non-empty
and if missing return c.Status(fiber.StatusBadRequest).SendString("missing
path") (or similar) before calling h.service.Stream so malformed requests are
rejected with 400; reference the variables rawPath/path and the h.service.Stream
call to locate where to insert the check.
In `@views/importing/queue_items_grouped_album.html`:
- Around line 131-169: The play/toggle controls are missing explicit button
types and the range sliders are unlabeled for accessibility; add type="button"
to every control <button> (both the toggle buttons using the hyperscript
attributes like the ones targeting "`#pa-queue-`{{.ID}}" and "`#pa-lib-`{{.ID}}" and
the play buttons that call toggleAudioPlayer(this) and have class "play-btn")
and add accessible labels to the <input type="range"> elements (e.g.,
aria-label="Playback position" or aria-labelledby pointing to a visible label,
and include aria-valuemin/aria-valuemax if appropriate) so seekAudioProgress and
updateAudioProgress work with labeled controls.
In `@views/importing/queue_items_grouped_artist.html`:
- Around line 129-134: Add explicit type="button" attributes to the audio
control buttons to prevent accidental form submission: update each <button>
element that uses the stimulus-like "_=" click toggles (e.g., the buttons
toggling .hidden on `#pg-queue-`{{.ID}} and `#pg-lib-`{{.ID}} and the other similar
buttons at the other noted locations) to include type="button". Locate the
button elements that have attributes like '_="on click toggle .hidden on
`#pg-queue-`{{.ID}}"' and add the type attribute to each so they don't act as
submit buttons when this fragment is inside a form.
- Around line 144-145: The template is exposing server filesystem paths via
Track.Path in the audio src (src="/stream?path={{urlEncode .Track.Path}}");
replace these with an opaque identifier (e.g. .Track.ID or .QueueItem.ID) in all
occurrences (lines around the existing audio tag and the other reported spots)
and change the client URL to use that id (e.g. /stream?id=...). Then update the
server handler (the Service.Stream call / stream route) to accept the opaque id,
perform a lookup by Track.ID or QueueItem.ID to resolve the filesystem path
server-side, and stream the file from the resolved path; ensure urlEncode usage
is applied to the id if needed and remove direct exposure of .Track.Path from
the template.
In `@views/importing/queue_items.html`:
- Around line 51-90: Add explicit type="button" to every <button> used for the
audio controls (both the toggle buttons that target `#player-queue-`{{.ID}} and
`#player-lib-`{{.ID}} and the play buttons that call toggleAudioPlayer(this)) to
prevent accidental form submission, and give each range input (those with
oninput="seekAudioProgress(this)") an accessible name by adding an aria-label or
aria-labelledby (e.g., "Seek playback" or a unique label per player using
{{.ID}}) so screen readers can identify the slider.
In `@views/library/track_overview_panel.html`:
- Around line 29-31: The range input used for audio seeking lacks an accessible
label; add an explicit label for the slider by giving the input a unique id
(e.g., seek-slider) and either add a visible <label for="seek-slider">Seek
audio</label> or an aria-label/aria-labelledby attribute (e.g., aria-label="Seek
audio") so screen readers can identify the control referenced by the
seekAudioProgress handler; ensure the label text clearly describes the control
and that the input retains its existing attributes and
oninput="seekAudioProgress(this)".
In `@views/partials/scripts.html`:
- Around line 155-157: The UI sets icon.className to the pause icon immediately
after calling audio.play(), which leaves the UI inconsistent if playback is
rejected; change the flow so audio.play()'s returned Promise is handled: call
audio.play().then(() => set icon.className = 'fas fa-pause fa-xs') and in
.catch(() => reset icon.className to the play icon (and optionally surface/log
the error)), ensuring the icon is updated only on successful play and restored
on failure to avoid unhandled promise rejections.
In `@views/partials/sidebar.html`:
- Around line 167-168: The sidebar element with id "active-jobs-badge" is still
polling the old endpoint via its hx-get attribute; update the hx-get value from
"/ui/jobs/count?filter=active" to the migrated path "/jobs/count?filter=active"
in views/partials/sidebar.html (preserve the existing hx-trigger, hx-push-url,
and hx-target attributes).
In `@views/tag/edit_form.html`:
- Around line 128-131: The range input used for audio seeking is missing an
accessible name, so update the input element (the range slider tied to the
seekAudioProgress function) to provide one — either add an aria-label like "Seek
audio" or add aria-labelledby pointing to a nearby visible label/span that
describes the control (e.g., the time-display span or a new label element);
ensure the label text clearly indicates it's the audio seek slider so screen
readers announce context when the slider is focused.
---
Outside diff comments:
In `@src/main.go`:
- Around line 137-150: The code currently calls server.Start() twice causing a
blocking start and potential address-in-use errors; remove the first blocking
call to server.Start() (the top-level call before signal handling) and instead
set up signal handling (quit channel and signal.Notify) and serverErr channel
first, then start the Fiber server exactly once inside the goroutine (where
server.Start() is already invoked), using the existing serverErr and quit
channels to handle shutdown and errors; ensure you still log the startup via
slog.Info and perform graceful shutdown (server.Shutdown or equivalent) when
quit or an error is received.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f6e19a4c-4998-43f0-adeb-7b1818d533f8
📒 Files selected for processing (16)
src/features/hosting/server.gosrc/features/importing/service.gosrc/features/library/service.gosrc/features/streaming/handlers.gosrc/features/streaming/routes.gosrc/features/streaming/service.gosrc/main.goviews/importing/queue_header.htmlviews/importing/queue_items.htmlviews/importing/queue_items_grouped_album.htmlviews/importing/queue_items_grouped_artist.htmlviews/library/track_overview_panel.htmlviews/partials/navbar.htmlviews/partials/scripts.htmlviews/partials/sidebar.htmlviews/tag/edit_form.html
Comment on lines
+397
to
+403
| track, err := s.library.GetTrack(ctx, trackID) | ||
| if err != nil { | ||
| return "", fmt.Errorf("track not found: %w", err) | ||
| } | ||
| if track == nil { | ||
| return "", fmt.Errorf("track not found") | ||
| } |
There was a problem hiding this comment.
Do not map every GetTrack failure to track not found.
This masks real storage/DB failures as not-found and can cause incorrect downstream 404 behavior. Keep not found for nil track only; propagate real lookup errors.
💡 Proposed fix
func (s *Service) GetLibraryTrackPath(ctx context.Context, trackID string) (string, error) {
track, err := s.library.GetTrack(ctx, trackID)
if err != nil {
- return "", fmt.Errorf("track not found: %w", err)
+ return "", fmt.Errorf("failed to load track %s: %w", trackID, err)
}
if track == nil {
- return "", fmt.Errorf("track not found")
+ return "", fmt.Errorf("track not found: %s", trackID)
}
return track.Path, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| track, err := s.library.GetTrack(ctx, trackID) | |
| if err != nil { | |
| return "", fmt.Errorf("track not found: %w", err) | |
| } | |
| if track == nil { | |
| return "", fmt.Errorf("track not found") | |
| } | |
| func (s *Service) GetLibraryTrackPath(ctx context.Context, trackID string) (string, error) { | |
| track, err := s.library.GetTrack(ctx, trackID) | |
| if err != nil { | |
| return "", fmt.Errorf("failed to load track %s: %w", trackID, err) | |
| } | |
| if track == nil { | |
| return "", fmt.Errorf("track not found: %s", trackID) | |
| } | |
| return track.Path, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/library/service.go` around lines 397 - 403, The current error
handling maps any error from s.library.GetTrack(ctx, trackID) to "track not
found", masking real storage/DB errors; change it so that if err != nil you
return the wrapped err (e.g., fmt.Errorf("failed to get track: %w", err)) and
only return the "track not found" error when track == nil; update the block
around the GetTrack call (referencing s.library.GetTrack, trackID and track) to
propagate real lookup errors and reserve the not-found message solely for a nil
track.
Comment on lines
+22
to
+31
| rawPath := c.Query("path") | ||
| path, err := url.QueryUnescape(rawPath) | ||
| if err != nil { | ||
| return c.Status(fiber.StatusBadRequest).SendString("invalid path") | ||
| } | ||
| resolved, mimeType, err := h.service.Stream(path) | ||
| if err != nil { | ||
| slog.Error("Stream: rejected path", "path", path, "error", err) | ||
| return c.Status(fiber.StatusNotFound).SendString("track not found") | ||
| } |
There was a problem hiding this comment.
Return 400 when path is missing before calling the service.
An empty/missing query currently falls through and is reported as 404 track not found, which misclassifies a malformed request.
💡 Proposed fix
func (h *Handler) Stream(c *fiber.Ctx) error {
rawPath := c.Query("path")
+ if rawPath == "" {
+ return c.Status(fiber.StatusBadRequest).SendString("missing path")
+ }
path, err := url.QueryUnescape(rawPath)
if err != nil {
return c.Status(fiber.StatusBadRequest).SendString("invalid path")
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/streaming/handlers.go` around lines 22 - 31, The handler
currently calls h.service.Stream even when the "path" query is empty, causing a
404 instead of a 400; update the code that reads rawPath/path to validate that
rawPath (or the decoded path) is non-empty and if missing return
c.Status(fiber.StatusBadRequest).SendString("missing path") (or similar) before
calling h.service.Stream so malformed requests are rejected with 400; reference
the variables rawPath/path and the h.service.Stream call to locate where to
insert the check.
Comment on lines
+131
to
+169
| <button _="on click toggle .hidden on #pa-queue-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-violet-500/10 border border-violet-400/30 text-violet-600 dark:text-violet-300 hover:bg-violet-500/20 transition-colors"> | ||
| <i class="fas fa-music fa-xs"></i> New | ||
| </button> | ||
| <button _="on click toggle .hidden on #pa-lib-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-blue-500/10 border border-blue-400/30 text-blue-600 dark:text-blue-300 hover:bg-blue-500/20 transition-colors"> | ||
| <i class="fas fa-music fa-xs"></i> Existing | ||
| </button> | ||
| </div> | ||
| <div id="pa-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2 bg-violet-500/5 border border-violet-400/20 rounded-lg px-2.5 py-1.5 w-full"> | ||
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-5 h-5 flex items-center justify-center rounded-full bg-violet-500/20 hover:bg-violet-500/40 text-violet-600 dark:text-violet-300 transition-colors"> | ||
| <i class="fas fa-play fa-xs"></i> | ||
| </button> | ||
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[#463371] mx-1" value="0" min="0" max="100" step="0.1"> | ||
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | ||
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | ||
| </div> | ||
| <div id="pa-lib-{{.ID}}" class="audio-player hidden flex items-center gap-2 bg-blue-500/5 border border-blue-400/20 rounded-lg px-2.5 py-1.5 w-full"> | ||
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-5 h-5 flex items-center justify-center rounded-full bg-blue-500/20 hover:bg-blue-500/40 text-blue-600 dark:text-blue-300 transition-colors"> | ||
| <i class="fas fa-play fa-xs"></i> | ||
| </button> | ||
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[#7BAADF] mx-1" value="0" min="0" max="100" step="0.1"> | ||
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | ||
| <audio src="/stream?path={{urlEncode (index .ItemMetadata "duplicate_path")}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | ||
| </div> | ||
| {{else}} | ||
| <div class="flex items-center gap-1.5"> | ||
| <button _="on click toggle .hidden on #pa-queue-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-gray-500/10 border border-gray-400/30 text-gray-600 dark:text-gray-300 hover:bg-gray-500/20 transition-colors"> | ||
| <i class="fas fa-music fa-xs"></i> Play | ||
| </button> | ||
| </div> | ||
| <div id="pa-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2 bg-gray-500/5 border border-gray-400/20 rounded-lg px-2.5 py-1.5 w-full"> | ||
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-5 h-5 flex items-center justify-center rounded-full bg-gray-500/20 hover:bg-gray-500/40 text-gray-600 dark:text-gray-300 transition-colors"> | ||
| <i class="fas fa-play fa-xs"></i> | ||
| </button> | ||
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1"> | ||
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | ||
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> |
There was a problem hiding this comment.
Apply explicit button types and slider labels in grouped-album players.
The new player controls have the same semantic/accessibility gaps: missing type="button" on control buttons and unlabeled range inputs.
💡 Proposed fix
- <button _="on click toggle .hidden on `#pa-queue-`{{.ID}}"
+ <button type="button" _="on click toggle .hidden on `#pa-queue-`{{.ID}}"
class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-violet-500/10 border border-violet-400/30 text-violet-600 dark:text-violet-300 hover:bg-violet-500/20 transition-colors">
@@
- <button _="on click toggle .hidden on `#pa-lib-`{{.ID}}"
+ <button type="button" _="on click toggle .hidden on `#pa-lib-`{{.ID}}"
class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-blue-500/10 border border-blue-400/30 text-blue-600 dark:text-blue-300 hover:bg-blue-500/20 transition-colors">
@@
- <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#463371`] mx-1" value="0" min="0" max="100" step="0.1">
+ <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#463371`] mx-1" value="0" min="0" max="100" step="0.1">
@@
- <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#7BAADF`] mx-1" value="0" min="0" max="100" step="0.1">
+ <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#7BAADF`] mx-1" value="0" min="0" max="100" step="0.1">
@@
- <button _="on click toggle .hidden on `#pa-queue-`{{.ID}}"
+ <button type="button" _="on click toggle .hidden on `#pa-queue-`{{.ID}}"
class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-gray-500/10 border border-gray-400/30 text-gray-600 dark:text-gray-300 hover:bg-gray-500/20 transition-colors">
@@
- <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1">
+ <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1">🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 131-131: The type attribute must be present on elements.
(button-type-require)
[warning] 135-135: The type attribute must be present on
elements.(button-type-require)
[warning] 141-141: The type attribute must be present on
elements.(button-type-require)
[warning] 149-149: The type attribute must be present on
elements.(button-type-require)
[error] 154-154: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 154-154: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 154-154: Tag must be paired, no start tag: [ ]
(tag-pair)
[warning] 158-158: The type attribute must be present on
elements.(button-type-require)
[error] 163-163: The id value [ pa-queue-{{.ID}} ] must be unique.
(id-unique)
[warning] 164-164: The type attribute must be present on
elements.(button-type-require)
[warning] 144-144: No matching [ label ] tag found.
(input-requires-label)
[warning] 152-152: No matching [ label ] tag found.
(input-requires-label)
[warning] 167-167: No matching [ label ] tag found.
(input-requires-label)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/importing/queue_items_grouped_album.html` around lines 131 - 169, The
play/toggle controls are missing explicit button types and the range sliders are
unlabeled for accessibility; add type="button" to every control <button> (both
the toggle buttons using the hyperscript attributes like the ones targeting
"`#pa-queue-`{{.ID}}" and "`#pa-lib-`{{.ID}}" and the play buttons that call
toggleAudioPlayer(this) and have class "play-btn") and add accessible labels to
the <input type="range"> elements (e.g., aria-label="Playback position" or
aria-labelledby pointing to a visible label, and include
aria-valuemin/aria-valuemax if appropriate) so seekAudioProgress and
updateAudioProgress work with labeled controls.
Source: Linters/SAST tools
Comment on lines
+129
to
+134
| <button _="on click toggle .hidden on #pg-queue-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-violet-500/10 border border-violet-400/30 text-violet-600 dark:text-violet-300 hover:bg-violet-500/20 transition-colors"> | ||
| <i class="fas fa-music fa-xs"></i> New | ||
| </button> | ||
| <button _="on click toggle .hidden on #pg-lib-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2 py-0.5 rounded text-xs font-medium bg-blue-500/10 border border-blue-400/30 text-blue-600 dark:text-blue-300 hover:bg-blue-500/20 transition-colors"> |
There was a problem hiding this comment.
Add explicit button types for the new audio controls.
These buttons should use type="button" to avoid accidental form submission behavior when this fragment is composed inside form contexts.
Suggested fix
-<button _="on click toggle .hidden on `#pg-queue-`{{.ID}}"
+<button type="button" _="on click toggle .hidden on `#pg-queue-`{{.ID}}"
...
-<button _="on click toggle .hidden on `#pg-lib-`{{.ID}}"
+<button type="button" _="on click toggle .hidden on `#pg-lib-`{{.ID}}"
...
-<button onclick="toggleAudioPlayer(this)" class="play-btn ...
+<button type="button" onclick="toggleAudioPlayer(this)" class="play-btn ...
...
-<button onclick="toggleAudioPlayer(this)" class="play-btn ...
+<button type="button" onclick="toggleAudioPlayer(this)" class="play-btn ...
...
-<button _="on click toggle .hidden on `#pg-queue-`{{.ID}}"
+<button type="button" _="on click toggle .hidden on `#pg-queue-`{{.ID}}"
...
-<button onclick="toggleAudioPlayer(this)" class="play-btn ...
+<button type="button" onclick="toggleAudioPlayer(this)" class="play-btn ...Also applies to: 139-139, 147-147, 156-156, 162-162
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 129-129: The type attribute must be present on elements.
(button-type-require)
[warning] 133-133: The type attribute must be present on
elements.(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/importing/queue_items_grouped_artist.html` around lines 129 - 134, Add
explicit type="button" attributes to the audio control buttons to prevent
accidental form submission: update each <button> element that uses the
stimulus-like "_=" click toggles (e.g., the buttons toggling .hidden on
`#pg-queue-`{{.ID}} and `#pg-lib-`{{.ID}} and the other similar buttons at the other
noted locations) to include type="button". Locate the button elements that have
attributes like '_="on click toggle .hidden on `#pg-queue-`{{.ID}}"' and add the
type attribute to each so they don't act as submit buttons when this fragment is
inside a form.
Source: Linters/SAST tools
Comment on lines
+144
to
+145
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | ||
| </div> |
There was a problem hiding this comment.
Avoid exposing filesystem paths in client-side stream URLs.
Embedding absolute track paths in src="/stream?path=..." leaks server filesystem structure to the browser, network logs, and intermediaries. Use opaque IDs in the UI and resolve paths server-side (queue item ID / track ID lookup) before calling Service.Stream.
Also applies to: 152-153, 167-168
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/importing/queue_items_grouped_artist.html` around lines 144 - 145, The
template is exposing server filesystem paths via Track.Path in the audio src
(src="/stream?path={{urlEncode .Track.Path}}"); replace these with an opaque
identifier (e.g. .Track.ID or .QueueItem.ID) in all occurrences (lines around
the existing audio tag and the other reported spots) and change the client URL
to use that id (e.g. /stream?id=...). Then update the server handler (the
Service.Stream call / stream route) to accept the opaque id, perform a lookup by
Track.ID or QueueItem.ID to resolve the filesystem path server-side, and stream
the file from the resolved path; ensure urlEncode usage is applied to the id if
needed and remove direct exposure of .Track.Path from the template.
Comment on lines
+51
to
+90
| <button _="on click toggle .hidden on #player-queue-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-violet-500/10 border border-violet-400/30 text-violet-600 dark:text-violet-300 hover:bg-violet-500/20 transition-colors"> | ||
| <i class="fas fa-music fa-xs"></i> New | ||
| </button> | ||
| <button _="on click toggle .hidden on #player-lib-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-blue-500/10 border border-blue-400/30 text-blue-600 dark:text-blue-300 hover:bg-blue-500/20 transition-colors"> | ||
| <i class="fas fa-music fa-xs"></i> Existing | ||
| </button> | ||
| </div> | ||
| <div id="player-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-violet-500/5 border border-violet-400/20 rounded-lg px-3 py-2 w-full"> | ||
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-violet-500/20 hover:bg-violet-500/40 text-violet-600 dark:text-violet-300 transition-colors"> | ||
| <i class="fas fa-play fa-xs"></i> | ||
| </button> | ||
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[#463371] mx-1" value="0" min="0" max="100" step="0.1"> | ||
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | ||
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | ||
| </div> | ||
| <div id="player-lib-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-blue-500/5 border border-blue-400/20 rounded-lg px-3 py-2 w-full"> | ||
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-blue-500/20 hover:bg-blue-500/40 text-blue-600 dark:text-blue-300 transition-colors"> | ||
| <i class="fas fa-play fa-xs"></i> | ||
| </button> | ||
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[#7BAADF] mx-1" value="0" min="0" max="100" step="0.1"> | ||
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | ||
| <audio src="/stream?path={{urlEncode (index .ItemMetadata "duplicate_path")}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | ||
| </div> | ||
| {{else}} | ||
| <div class="flex items-center gap-2"> | ||
| <button _="on click toggle .hidden on #player-queue-{{.ID}}" | ||
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-gray-500/10 border border-gray-400/30 text-gray-600 dark:text-gray-300 hover:bg-gray-500/20 transition-colors"> | ||
| <i class="fas fa-music fa-xs"></i> Play | ||
| </button> | ||
| </div> | ||
| <div id="player-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-gray-500/5 border border-gray-400/20 rounded-lg px-3 py-2 w-full"> | ||
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-gray-500/20 hover:bg-gray-500/40 text-gray-600 dark:text-gray-300 transition-colors"> | ||
| <i class="fas fa-play fa-xs"></i> | ||
| </button> | ||
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1"> | ||
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | ||
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | ||
| </div> |
There was a problem hiding this comment.
Set explicit button types and label the audio sliders.
The new control buttons should be type="button" to avoid accidental form submission, and each range input needs an accessible name.
💡 Proposed fix
- <button _="on click toggle .hidden on `#player-queue-`{{.ID}}"
+ <button type="button" _="on click toggle .hidden on `#player-queue-`{{.ID}}"
class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-violet-500/10 border border-violet-400/30 text-violet-600 dark:text-violet-300 hover:bg-violet-500/20 transition-colors">
@@
- <button _="on click toggle .hidden on `#player-lib-`{{.ID}}"
+ <button type="button" _="on click toggle .hidden on `#player-lib-`{{.ID}}"
class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-blue-500/10 border border-blue-400/30 text-blue-600 dark:text-blue-300 hover:bg-blue-500/20 transition-colors">
@@
- <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#463371`] mx-1" value="0" min="0" max="100" step="0.1">
+ <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#463371`] mx-1" value="0" min="0" max="100" step="0.1">
@@
- <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#7BAADF`] mx-1" value="0" min="0" max="100" step="0.1">
+ <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#7BAADF`] mx-1" value="0" min="0" max="100" step="0.1">
@@
- <button _="on click toggle .hidden on `#player-queue-`{{.ID}}"
+ <button type="button" _="on click toggle .hidden on `#player-queue-`{{.ID}}"
class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-gray-500/10 border border-gray-400/30 text-gray-600 dark:text-gray-300 hover:bg-gray-500/20 transition-colors">
@@
- <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1">
+ <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button _="on click toggle .hidden on #player-queue-{{.ID}}" | |
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-violet-500/10 border border-violet-400/30 text-violet-600 dark:text-violet-300 hover:bg-violet-500/20 transition-colors"> | |
| <i class="fas fa-music fa-xs"></i> New | |
| </button> | |
| <button _="on click toggle .hidden on #player-lib-{{.ID}}" | |
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-blue-500/10 border border-blue-400/30 text-blue-600 dark:text-blue-300 hover:bg-blue-500/20 transition-colors"> | |
| <i class="fas fa-music fa-xs"></i> Existing | |
| </button> | |
| </div> | |
| <div id="player-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-violet-500/5 border border-violet-400/20 rounded-lg px-3 py-2 w-full"> | |
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-violet-500/20 hover:bg-violet-500/40 text-violet-600 dark:text-violet-300 transition-colors"> | |
| <i class="fas fa-play fa-xs"></i> | |
| </button> | |
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[#463371] mx-1" value="0" min="0" max="100" step="0.1"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | |
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | |
| </div> | |
| <div id="player-lib-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-blue-500/5 border border-blue-400/20 rounded-lg px-3 py-2 w-full"> | |
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-blue-500/20 hover:bg-blue-500/40 text-blue-600 dark:text-blue-300 transition-colors"> | |
| <i class="fas fa-play fa-xs"></i> | |
| </button> | |
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[#7BAADF] mx-1" value="0" min="0" max="100" step="0.1"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | |
| <audio src="/stream?path={{urlEncode (index .ItemMetadata "duplicate_path")}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | |
| </div> | |
| {{else}} | |
| <div class="flex items-center gap-2"> | |
| <button _="on click toggle .hidden on #player-queue-{{.ID}}" | |
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-gray-500/10 border border-gray-400/30 text-gray-600 dark:text-gray-300 hover:bg-gray-500/20 transition-colors"> | |
| <i class="fas fa-music fa-xs"></i> Play | |
| </button> | |
| </div> | |
| <div id="player-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-gray-500/5 border border-gray-400/20 rounded-lg px-3 py-2 w-full"> | |
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-gray-500/20 hover:bg-gray-500/40 text-gray-600 dark:text-gray-300 transition-colors"> | |
| <i class="fas fa-play fa-xs"></i> | |
| </button> | |
| <input type="range" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | |
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | |
| </div> | |
| <button type="button" _="on click toggle .hidden on `#player-queue-`{{.ID}}" | |
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-violet-500/10 border border-violet-400/30 text-violet-600 dark:text-violet-300 hover:bg-violet-500/20 transition-colors"> | |
| <i class="fas fa-music fa-xs"></i> New | |
| </button> | |
| <button type="button" _="on click toggle .hidden on `#player-lib-`{{.ID}}" | |
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-blue-500/10 border border-blue-400/30 text-blue-600 dark:text-blue-300 hover:bg-blue-500/20 transition-colors"> | |
| <i class="fas fa-music fa-xs"></i> Existing | |
| </button> | |
| </div> | |
| <div id="player-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-violet-500/5 border border-violet-400/20 rounded-lg px-3 py-2 w-full"> | |
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-violet-500/20 hover:bg-violet-500/40 text-violet-600 dark:text-violet-300 transition-colors"> | |
| <i class="fas fa-play fa-xs"></i> | |
| </button> | |
| <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#463371`] mx-1" value="0" min="0" max="100" step="0.1"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | |
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | |
| </div> | |
| <div id="player-lib-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-blue-500/5 border border-blue-400/20 rounded-lg px-3 py-2 w-full"> | |
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-blue-500/20 hover:bg-blue-500/40 text-blue-600 dark:text-blue-300 transition-colors"> | |
| <i class="fas fa-play fa-xs"></i> | |
| </button> | |
| <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-[`#7BAADF`] mx-1" value="0" min="0" max="100" step="0.1"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | |
| <audio src="/stream?path={{urlEncode (index .ItemMetadata "duplicate_path")}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | |
| </div> | |
| {{else}} | |
| <div class="flex items-center gap-2"> | |
| <button type="button" _="on click toggle .hidden on `#player-queue-`{{.ID}}" | |
| class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-md text-xs font-medium bg-gray-500/10 border border-gray-400/30 text-gray-600 dark:text-gray-300 hover:bg-gray-500/20 transition-colors"> | |
| <i class="fas fa-music fa-xs"></i> Play | |
| </button> | |
| </div> | |
| <div id="player-queue-{{.ID}}" class="audio-player hidden flex items-center gap-2.5 bg-gray-500/5 border border-gray-400/20 rounded-lg px-3 py-2 w-full"> | |
| <button onclick="toggleAudioPlayer(this)" class="play-btn flex-shrink-0 w-6 h-6 flex items-center justify-center rounded-full bg-gray-500/20 hover:bg-gray-500/40 text-gray-600 dark:text-gray-300 transition-colors"> | |
| <i class="fas fa-play fa-xs"></i> | |
| </button> | |
| <input type="range" aria-label="Seek audio position" oninput="seekAudioProgress(this)" class="flex-1 h-1 rounded-full cursor-pointer accent-gray-500 mx-1" value="0" min="0" max="100" step="0.1"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 tabular-nums w-8 text-right flex-shrink-0">0:00</span> | |
| <audio src="/stream?path={{urlEncode .Track.Path}}" preload="none" ontimeupdate="updateAudioProgress(this)" onended="resetAudioPlayer(this)"></audio> | |
| </div> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 51-51: The type attribute must be present on elements.
(button-type-require)
[warning] 55-55: The type attribute must be present on
elements.(button-type-require)
[warning] 61-61: The type attribute must be present on
elements.(button-type-require)
[warning] 69-69: The type attribute must be present on
elements.(button-type-require)
[error] 74-74: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 74-74: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 74-74: Tag must be paired, no start tag: [ ]
(tag-pair)
[warning] 78-78: The type attribute must be present on
elements.(button-type-require)
[error] 83-83: The id value [ player-queue-{{.ID}} ] must be unique.
(id-unique)
[warning] 84-84: The type attribute must be present on
elements.(button-type-require)
[warning] 64-64: No matching [ label ] tag found.
(input-requires-label)
[warning] 72-72: No matching [ label ] tag found.
(input-requires-label)
[warning] 87-87: No matching [ label ] tag found.
(input-requires-label)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/importing/queue_items.html` around lines 51 - 90, Add explicit
type="button" to every <button> used for the audio controls (both the toggle
buttons that target `#player-queue-`{{.ID}} and `#player-lib-`{{.ID}} and the play
buttons that call toggleAudioPlayer(this)) to prevent accidental form
submission, and give each range input (those with
oninput="seekAudioProgress(this)") an accessible name by adding an aria-label or
aria-labelledby (e.g., "Seek playback" or a unique label per player using
{{.ID}}) so screen readers can identify the slider.
Source: Linters/SAST tools
Comment on lines
+29
to
+31
| <input type="range" min="0" max="100" value="0" step="0.1" | ||
| class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer" | ||
| oninput="seekAudioProgress(this)"> |
There was a problem hiding this comment.
Add an accessible label to the audio seek slider.
Line 29 introduces a range input without an associated label, which breaks accessible form semantics.
Suggested fix
- <input type="range" min="0" max="100" value="0" step="0.1"
+ <input type="range" min="0" max="100" value="0" step="0.1"
+ aria-label="Audio playback position"
class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer"
oninput="seekAudioProgress(this)">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input type="range" min="0" max="100" value="0" step="0.1" | |
| class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer" | |
| oninput="seekAudioProgress(this)"> | |
| <input type="range" min="0" max="100" value="0" step="0.1" | |
| aria-label="Audio playback position" | |
| class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer" | |
| oninput="seekAudioProgress(this)"> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 29-29: No matching [ label ] tag found.
(input-requires-label)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/library/track_overview_panel.html` around lines 29 - 31, The range
input used for audio seeking lacks an accessible label; add an explicit label
for the slider by giving the input a unique id (e.g., seek-slider) and either
add a visible <label for="seek-slider">Seek audio</label> or an
aria-label/aria-labelledby attribute (e.g., aria-label="Seek audio") so screen
readers can identify the control referenced by the seekAudioProgress handler;
ensure the label text clearly describes the control and that the input retains
its existing attributes and oninput="seekAudioProgress(this)".
Source: Linters/SAST tools
Comment on lines
+155
to
+157
| audio.play(); | ||
| icon.className = 'fas fa-pause fa-xs'; | ||
| } else { |
There was a problem hiding this comment.
Handle audio.play() rejection to keep UI state consistent.
If playback fails, the icon stays on pause even though nothing is playing. Update icon only on successful play and reset on catch.
Suggested fix
- audio.play();
- icon.className = 'fas fa-pause fa-xs';
+ audio.play()
+ .then(function() {
+ icon.className = 'fas fa-pause fa-xs';
+ })
+ .catch(function() {
+ icon.className = 'fas fa-play fa-xs';
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| audio.play(); | |
| icon.className = 'fas fa-pause fa-xs'; | |
| } else { | |
| audio.play() | |
| .then(function() { | |
| icon.className = 'fas fa-pause fa-xs'; | |
| }) | |
| .catch(function() { | |
| icon.className = 'fas fa-play fa-xs'; | |
| }); | |
| } else { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/partials/scripts.html` around lines 155 - 157, The UI sets
icon.className to the pause icon immediately after calling audio.play(), which
leaves the UI inconsistent if playback is rejected; change the flow so
audio.play()'s returned Promise is handled: call audio.play().then(() => set
icon.className = 'fas fa-pause fa-xs') and in .catch(() => reset icon.className
to the play icon (and optionally surface/log the error)), ensuring the icon is
updated only on successful play and restored on failure to avoid unhandled
promise rejections.
Comment on lines
+128
to
+131
| <input type="range" min="0" max="100" value="0" step="0.1" | ||
| class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer" | ||
| oninput="seekAudioProgress(this)"> | ||
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 w-8 text-right tabular-nums">0:00</span> |
There was a problem hiding this comment.
Add an accessible name to the seek slider.
The range input is unlabeled, so assistive tech announces it without context.
💡 Proposed fix
- <input type="range" min="0" max="100" value="0" step="0.1"
+ <input type="range" min="0" max="100" value="0" step="0.1"
+ aria-label="Seek audio position"
class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer"
oninput="seekAudioProgress(this)">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input type="range" min="0" max="100" value="0" step="0.1" | |
| class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer" | |
| oninput="seekAudioProgress(this)"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 w-8 text-right tabular-nums">0:00</span> | |
| <input type="range" min="0" max="100" value="0" step="0.1" | |
| aria-label="Seek audio position" | |
| class="flex-1 h-1 mx-1 accent-gray-500 cursor-pointer" | |
| oninput="seekAudioProgress(this)"> | |
| <span class="time-display text-xs text-gray-500 dark:text-gray-400 w-8 text-right tabular-nums">0:00</span> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 128-128: No matching [ label ] tag found.
(input-requires-label)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/tag/edit_form.html` around lines 128 - 131, The range input used for
audio seeking is missing an accessible name, so update the input element (the
range slider tied to the seekAudioProgress function) to provide one — either add
an aria-label like "Seek audio" or add aria-labelledby pointing to a nearby
visible label/span that describes the control (e.g., the time-display span or a
new label element); ensure the label text clearly indicates it's the audio seek
slider so screen readers announce context when the slider is focused.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wip
Summary by CodeRabbit
Documentation
New Features
Improvements