mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-26 06:42:32 +00:00
8d3db5f42c
Some checks are pending
ship-ci / ci (push) Waiting to run
* fix(nav): the library sometimes showed the legacy screen — map 'home' inside showScreen
Testers: "randomly, when moving to the library from another menu option, the library shows the
old interface — never when a song ends."
━━━ WHAT WAS ACTUALLY HAPPENING ━━━
#home is the PRE-V3 library screen. The v3 shell replaced it with #v3-songs, and the mapping DID
exist — but only inside WRAPPERS on window.showScreen, and only for callers that go through
`window`. THREE independent parties monkey-patch it, each capturing whatever happens to be there
at the time:
app.js publishes the raw function
-> shell.js wraps it, adding the home -> v3-songs mapping
-> the stems plugin wraps it AGAIN (src/main.js:1029), capturing the current value
Plugins load ASYNCHRONOUSLY. The chain links up in whatever order the race settles, and any
capture taken before shell.js installs — or any re-assignment after it — silently drops the
mapping. Hence "randomly".
AND THE INTERNAL CALLERS NEVER TOUCHED window.showScreen AT ALL. closeCurrentSong and the
Esc-from-settings shortcut call the IMPORTED showScreen, which no wrapper ever sees. Reproduced
in a browser: the unwrapped function with 'home' lands on the dead legacy screen EVERY time.
"Never when a song ends" is the tell, and it is what identified the mechanism: closeCurrentSong
resolves its target through _resolvePlayerOrigin(), which ALREADY applies this mapping. That one
path was fine — which is exactly why the bug looked random rather than total.
PRE-EXISTING, not a regression from the module carve: the onclick="showScreen('home')" links and
the wrapper-only mapping both date to 2026-06-22.
━━━ THE FIX ━━━
The guard lives inside showScreen now: ONE place, in the function every caller routes through,
instead of a chain of monkey-patches that must each remember. Wrapper order stops mattering, and
the module-internal callers are covered for the first time.
Verified in a browser: the raw, unwrapped showScreen('home') — which reproduced as #home — now
lands on #v3-songs, and cannot be undone by any wrapper order.
━━━ AND A [P1] I INTRODUCED, WHICH CODEX CAUGHT ━━━
My first cut mapped BOTH 'home' and 'v3-home', copied straight from _resolvePlayerOrigin.
That is correct THERE and wrong HERE. _resolvePlayerOrigin computes where to RETURN TO after a
song, and landing on the Songs list from the dashboard is the right behaviour. But #v3-home is
the v3 DASHBOARD — a real screen that the shell's Home nav, the onboarding tour and the dashboard
re-render listener all target. Redirecting it would have made Home unreachable.
A LEGACY ALIAS IS NOT THE SAME THING AS A RETURN TARGET. Only 'home' is mapped now, and a test
pins that: re-adding 'v3-home' to the guard fails it.
4 tests, bite-tested both ways.
node 1049, pytest 2425, ESLint 0, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(nav): nobody may monkey-patch window.showScreen — add screen:changing, make the shell listen (#924)
window.showScreen was wrapped by THREE independent parties, each capturing whatever happened to be
there at the time:
app.js publishes the raw function
-> static/v3/shell.js wrapped it (to call syncActive, and to map home -> v3-songs)
-> the stems plugin wrapped it AGAIN (to tear down on leaving the player)
Plugins load ASYNCHRONOUSLY, so the chain linked up in whatever order the race settled. A capture
taken before shell.js installed silently dropped the mapping it carried — and the library opened on
the dead legacy #home screen. Testers saw that as "randomly, the library shows the old interface"
(#923).
#923 fixed the symptom by moving the mapping inside showScreen. This removes the CAUSE: neither
wrapper ever needed to be one.
━━━ TWO EVENTS, AND THE DISTINCTION IS THE WHOLE POINT ━━━
screen:changing emitted BEFORE anything happens. "I am leaving `from`." Teardown/cancel here.
screen:changed emitted after the DOM and data settle. "I am on `id`." Now carries `from`.
screen:changing is new, and it exists because Codex caught me collapsing the two. The stems plugin
tore down its audio graph BEFORE showScreen did anything; screen:changed fires at the very END,
after core awaits library and provider loads — so moving the plugin onto it would have delayed
teardown behind a slow fetch, or skipped it entirely if that fetch threw, and stems would keep
playing on a non-player screen. A test pins the ordering: screen:changing must precede the first
await.
shell.js is a plain screen:changed listener now, like app.js, audio-mixer.js and tour-engine.js
already were. window.showScreen is an unwrapped function again, and tests/js/
no_showscreen_monkeypatch.test.js fails CI if anything in static/ ever assigns to it again — so the
hazard is structurally impossible rather than merely avoided.
━━━ AND A FALLBACK THAT COULD NEVER FIRE ━━━
My retry-if-the-bus-is-late path listened for `slopsmith:capabilities:ready`. Core dispatches
`feedBack:capabilities:ready` (capabilities.js:1536) — the slopsmith: name is the PRE-DMCA event
and nothing has emitted it since the rename. Codex caught it. A guard that cannot fire is worse
than no guard: it reads as protection and is decoration.
(The same dead-event bug turned out to be sitting in THREE of the stems plugin's fallbacks, where
it has silently disabled its lifecycle wiring whenever the bus was late. Fixed in
feedback-plugin-stems#38.)
VERIFIED. A/B against origin/main: the nav highlight and topbar title follow IDENTICALLY with
shell.js as a listener; screen:changing -> screen:changed fire in order with the right {id, from};
window.showScreen is unwrapped; and showScreen('home') still lands on v3-songs.
node 1053, pytest 2425, ESLint 0, Codex 0.
Closes #924
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|---|---|---|
| .. | ||
| alpha_warning_banner.test.js | ||
| app_tour_library.test.js | ||
| audio_effects_host.test.js | ||
| audio_effects_test_harness.js | ||
| audio_element_accessor.test.js | ||
| audio_session_compat.test.js | ||
| audio_session_host.test.js | ||
| audio_session_input.test.js | ||
| audio_session_mix.test.js | ||
| audio_session_monitoring.test.js | ||
| audio_session_routes.test.js | ||
| audio_session_stems_bridge.test.js | ||
| audio_session_stems.test.js | ||
| audio_session_test_harness.js | ||
| autoplay_exit.test.js | ||
| badges_handedness.test.js | ||
| beats_loaded.test.js | ||
| capabilities_claims.test.js | ||
| capabilities_diagnostics.test.js | ||
| capabilities_lifecycle.test.js | ||
| capabilities_manifest_runtime.test.js | ||
| capabilities_multi_provider.test.js | ||
| capabilities_ownership.test.js | ||
| capabilities_test_harness.js | ||
| capabilities_versioning.test.js | ||
| capability_inspector_nav.test.js | ||
| capability_inspector_render.test.js | ||
| diagnostics_summary.test.js | ||
| drum_keys_highway_3d_resize_reframe.test.js | ||
| edit_metadata_modal.test.js | ||
| editor_pending_view.test.js | ||
| handshapes_pipeline.test.js | ||
| highway_3d_arp_deferral.test.js | ||
| highway_3d_camera_framing.test.js | ||
| highway_3d_context_loss.test.js | ||
| highway_3d_fret_label_render_order.test.js | ||
| highway_3d_fret_row_labels.test.js | ||
| highway_3d_fret_spacing.test.js | ||
| highway_3d_lean_sustain.test.js | ||
| highway_3d_lefty.test.js | ||
| highway_3d_overlay_full_cover.test.js | ||
| highway_3d_panel_controls.test.js | ||
| highway_3d_pool_warm.test.js | ||
| highway_3d_render_order.test.js | ||
| highway_3d_resize_reframe.test.js | ||
| highway_3d_slide_target.test.js | ||
| highway_3d_smooth_clock_pause.test.js | ||
| highway_3d_sustain_bloom.test.js | ||
| highway_3d_sustain_rail.test.js | ||
| highway_3d_wide_fov.test.js | ||
| highway_adaptive_scale.test.js | ||
| highway_audio_barrier.test.js | ||
| highway_bend_curve.test.js | ||
| highway_chord_harmony.test.js | ||
| highway_chord_render_cache.test.js | ||
| highway_colors_facade.test.js | ||
| highway_filtered_notes.test.js | ||
| highway_monotonic_clock.test.js | ||
| highway_note_state.test.js | ||
| highway_pause_throttle.test.js | ||
| highway_phrases.test.js | ||
| highway_renderer_swap_canvas_reset.test.js | ||
| highway_string_colors.test.js | ||
| highway_teaching_marks.test.js | ||
| highway_visibility.test.js | ||
| host_contract.test.js | ||
| juce_engine_reroute.test.js | ||
| legacy_shim_hits.test.js | ||
| library_card_actions.test.js | ||
| live_guitar_tone_source.test.js | ||
| live_performance_hud.test.js | ||
| loop_api.test.js | ||
| loop_restart.test.js | ||
| midi_input_domain.test.js | ||
| no_showscreen_monkeypatch.test.js | ||
| note_detection_domain.test.js | ||
| pedal_cables.test.js | ||
| play_button_reroute_guard.test.js | ||
| play_queue_peek.test.js | ||
| play_queue_shuffle.test.js | ||
| playback_app_adapter.test.js | ||
| playback_compat.test.js | ||
| playback_diagnostics.test.js | ||
| playback_domain.test.js | ||
| playback_test_harness.js | ||
| plugin_hydration_wipe.test.js | ||
| plugin_loader_script_type.test.js | ||
| plugin_module_rollback.test.js | ||
| plugin_style_injection.test.js | ||
| plugins_page.test.js | ||
| progress_calibration.test.js | ||
| progression_notifications.test.js | ||
| progression_progress_events.test.js | ||
| renderer_bus_feeder.test.js | ||
| section_practice_dismiss.test.js | ||
| settings_autosave.test.js | ||
| show_screen_legacy_home.test.js | ||
| song_close.test.js | ||
| song_credits_overlay.test.js | ||
| song_event_payload.test.js | ||
| song_restart.test.js | ||
| song_seek.test.js | ||
| speed_reset.test.js | ||
| test_utils.js | ||
| tour_engine.test.js | ||
| tuner_auto_open.test.js | ||
| tuner_core.test.js | ||
| tuning_display.test.js | ||
| tuning_targets.test.js | ||
| v3_add_to_playlist_menu.test.js | ||
| v3_az_rail.test.js | ||
| v3_collections.test.js | ||
| v3_favorites_toggle.test.js | ||
| v3_keep_practicing.test.js | ||
| v3_library_refresh.test.js | ||
| v3_playlist_cover.test.js | ||
| v3_songs_score_badge_refresh.test.js | ||
| v3_songs_scroll.test.js | ||
| v3_songs_tuning.test.js | ||
| v3_songs_window_recycle.test.js | ||
| v3_theme_read_api.test.js | ||
| v3_user_select_policy.test.js | ||
| venue_instrument_pov.test.js | ||
| venue_mood_fx.test.js | ||
| venue_scene_3d.test.js | ||
| venue_viz.test.js | ||
| visualization_domain.test.js | ||
| window_contract.test.js | ||
| working_tuning_capability.test.js | ||
| working_tuning.test.js | ||