Skip to content

fix(linux): not_viewable tras show() se reintenta en silencio y los comentarios no prometen confirmaciones (#179) - #221

Merged
egdev6 merged 5 commits into
egdev6:mainfrom
Rafaeldelinares:fix/x11-placement-followup-205
Oct 31, 2026
Merged

egdev6 merged 5 commits into
egdev6:mainfrom
Rafaeldelinares:fix/x11-placement-followup-205

Conversation

@Rafaeldelinares

@Rafaeldelinares Rafaeldelinares commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Related issue / Issue relacionada

Issue: #179 (fix(linux): tolera el redondeo del gestor al leer el tamaño efectivo (#179), ya integrada en f71039b)
ODD: odd/tasks/x11-native-widget-resize.md — seguimiento de las dos notas no bloqueantes que dejó la review de #205 (#205). Encaja en #107 («Escritorio y widget Linux»), cuyo pendiente incluye el smoke de GNOME.

Scope / Alcance

Seguimiento de las dos notas de la review de #205. No cambia el comportamiento de la colocación: el ciclo nativo, el read-back, la tolerancia de ±1 px y los códigos de motivo quedan igual.

  1. Comentarios que prometen más de lo que el código garantiza (nota «comentarios que prometen más de lo que el código garantiza»):
    • rooted_origin se apoyaba en el retorno de gdk_window_get_origin como si dijera que la lectura es utilizable. GDK3 lo documenta como «not meaningful, ignore» y en la práctica vale 0 sólo con la ventana destruida: no detecta un fallo de traducción de coordenadas. El código no cambia (el 0 sigue siendo «no hay origen» y no se usa el respaldo relativo al padre); cambia lo que los comentarios afirman de él.
    • apply_size devolvía !is_resizable() justo después de set_resizable(false), que no puede fallar: el valor informa del estado del permiso al terminar, no de una confirmación. El comportamiento no cambia; el comentario deja de llamarlo confirmación.
    • Las mismas afirmaciones vivían en el artefacto ODD de bug(linux/x11): la tira no alcanza el borde inferior de la pantalla al arrastrarla #179: se corrigen también para que el documento no contradiga al código.
  2. not_viewable tras show() (nota «el presupuesto de lecturas es justo»): si el compositor tarda en mapear la ventana recién mostrada, el backend informaba not_viewable y la tira mostraba el reintento como un fallo que no había ocurrido. Ahora ese not_viewable se reintenta en silencio con un presupuesto acotado (5 lecturas, 100 ms entre ellas); sólo si se agota se muestra el fallo, y entonces sí es real. La política vive en una función pura y testeada (readPlacementWithRetries), sin dependencias nuevas y sin textos nuevos. A petición de la revisión, abandona inmediatamente si el usuario oculta la tira antes o durante la espera, y descarta respuestas de lecturas en vuelo tras la ocultación para evitar llamadas innecesarias y falsos fallos.

Fuera de alcance: el ciclo nativo de colocación, la geometría, los motivos, la tolerancia, la detección de transporte, el presupuesto de lecturas del backend (sigue siendo «justo») y cualquier cambio de comportamiento en Windows o macOS.

Coordination / Coordinación

  • Para ODD: asignaciones y solapamientos revisados; toma verificada o acuerdo explícito del mantenedor enlazado.
  • For ODD: issue/PR assignees and overlapping work checked; claim verified, or explicit maintainer coordination linked. — N/A (esta PR se responde en español).

Contributor / Contribuidor: @Rafaeldelinares
Claim or agreement evidence / Evidencia de asignación o acuerdo: las dos notas son las que dejó la review de #205 el 29-10-2026 (#205) y que el propio mantenedor propuso como seguimiento; el mantenedor integró #205 y cerró #179.
Use one language; mark non-ODD items N/A. / Usa un idioma; marca N/A si no aplica ODD.

Acceptance and validation / Aceptación y validación

  • Criteria satisfied / Criterios cumplidos:
    • Los comentarios de rooted_origin, apply_size y de su llamador ya no afirman una confirmación que el código no hace; el artefacto ODD de bug(linux/x11): la tira no alcanza el borde inferior de la pantalla al arrastrarla #179 queda coherente con el código.
    • not_viewable tras show() se reintenta en silencio: 5 lecturas con 100 ms entre ellas, y el fallo visible sólo aparece si se agota el presupuesto.
    • Abandona y descarta lecturas si el usuario oculta la tira durante los reintentos (shouldAbort: () => hiddenByUser.current).
    • El comportamiento terminal no cambia: si la ventana no se puede leer, el fallo se informa con su motivo y la tira ofrece el reintento.
    • Sin dependencias nuevas, sin textos nuevos (no hay cambios en los catálogos ES/EN) y sin tocar la capa nativa.
  • Checks actually run and results / Comprobaciones ejecutadas y resultados:
    • npx vitest run --dir src → 63 ficheros, 797 tests, 0 fallos (9 nuevos en src/lib/widget-placement.test.ts).
    • RED de los tests nuevos sin la implementación: fallan adecuadamente; GREEN con ella: 23 passed en ese fichero.
    • npm test (vitest + node --test) → vitest 797 ok + 298 node ok, 0 fallos.
    • npm run typecheck (tsc --noEmit) → limpio.
    • cargo test --locked → 384 ok / 0 fallos / 3 ignorados.
    • cargo fmt --manifest-path src-tauri/Cargo.toml --all -- --check → limpio.
    • node scripts/i18n-inventory.mjs --check → limpio (incluye la regeneración del inventario en commit 926bf9d).
    • Tamaño del diff acumulado en la PR: 6 ficheros.
  • Checks not run and why / Comprobaciones no ejecutadas y motivo:
  • Remaining blockers / Bloqueos pendientes:

An open PR does not mean integration. Release, secrets and signing operations require separate authorization.
Una PR abierta no implica integración. Release, secretos y firma requieren autorización separada.

… el código garantiza (egdev6#179)

Two comments claimed a confirmation the code does not make. `rooted_origin`
leaned on the return value of `gdk_window_get_origin` as if it proved the
read was usable; GDK3 documents that value as "not meaningful, ignore" and in
practice it is zero only for a destroyed window, so it never detects a
coordinate translation failure. `apply_size` returned `!is_resizable()` right
after `set_resizable(false)`, which cannot fail: the value reports the state
of the permission, not the success of the resize.

The behaviour does not change: a zero origin still means "no origin" and the
parent-relative fallback is still refused, and the cycle still requires the
permission to be closed. Only the comments change, plus the two sentences in
the egdev6#179 ODD artifact that repeated the same claims.
…dev6#179)

When the compositor takes longer to map the freshly shown window, the backend
reports `not_viewable` and the strip showed the retry as a failure that never
happened. The read is now retried silently with a bounded budget (five reads,
100 ms apart); the failure only surfaces when the budget is exhausted, and
then it is real.

The policy lives in a pure function (`readPlacementWithRetries`) covered by
deterministic tests, so it needs no X server. No new dependencies, no new
user-facing text and no change in the native placement cycle.
@egdev6

egdev6 commented Oct 31, 2026

Copy link
Copy Markdown
Owner

@Rafaeldelinares, revisé el head 926bf9dc3bb54fe1be03e99fb4ce773b24f95d6f. Los cambios Rust son solo comentarios, el helper tiene presupuesto acotado y no reintenta los otros motivos, y los cinco checks están verdes sobre este head. Antes de integrar pediría un ajuste en la cancelación:

Ocultar la tira durante la espera no detiene los reintentos. En src/Widget.tsx:571–577, hiddenByUser.current se comprueba antes de entrar al helper, pero el callback de cada intento solo comprueba disposed y current(). El listener de widget-hidden actualiza la referencia sin invalidar ese trabajo. Si el usuario oculta la tira durante una espera de 100 ms, continúan las llamadas a setWidgetBounds y, al agotarse el presupuesto, puede publicarse not_viewable como fallo aunque la ocultación haya sido voluntaria.

Por favor, abandona antes de cada intento si la tira fue ocultada y descarta también la respuesta si se ocultó durante la llamada. Añade una regresión que oculte durante la espera y compruebe que no hay más llamadas nativas ni publicación de fallo, además del caso de ocultación mientras una llamada está en vuelo. El test actual de abandono devuelve null en la primera lectura, por lo que no cubre esta transición.

Dos notas no bloqueantes: el smoke GNOME/Mutter y el compositor lento siguen pendientes, tal como declaras; y conviene actualizar el cuerpo, que dice cinco archivos y sin regeneración del inventario, cuando el diff contiene seis e incluye esa actualización. Revisión estática, sin ejecutar tests localmente.

… tira (egdev6#179)

Aborta el ciclo de lecturas silenciosas ante `not_viewable` si la tira
fue ocultada antes o durante la espera, y descarta respuestas de
llamadas que estuvieran en vuelo. Evita llamadas nativas innecesarias
y falsos reportes de fallo por ocultación voluntaria.
@Rafaeldelinares

Copy link
Copy Markdown
Contributor Author

@egdev6, aplicado el ajuste en el head c5bc060:

  • readPlacementWithRetries recibe ahora la opción shouldAbort?: () => boolean, evaluada antes de la primera llamada, antes de cada espera y tras cada lectura nativa.
  • En src/Widget.tsx, se enlaza con hiddenByUser.current: si el usuario oculta la tira mientras se espera entre reintentos o mientras una lectura está en vuelo, se cancelan los intentos restantes y se descarta el reporte sin llamar más a setWidgetBounds ni publicar fallo.
  • Añadidos tres tests de regresión en src/lib/widget-placement.test.ts que cubren: ocultación durante la espera (abandono inmediato), ocultación durante una lectura en vuelo (descarte del reporte) y abandono previo a la primera llamada.
  • Actualizado también el cuerpo de la PR con el recuento de 6 ficheros (que incluye la regeneración del inventario en 926bf9d).

Todos los tests en verde (npm test 797 vitest + 298 node, cargo test 384 ok, fmt e i18n limpios).

@egdev6

egdev6 commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Gracias, @Rafaeldelinares. Revisado el commit 332360d: no encontré hallazgos bloqueantes. El punto de la revisión anterior queda resuelto: los reintentos se cancelan al ocultar la tira y se descartan las respuestas en vuelo, con tests para ambos casos y para la cancelación antes de la primera lectura.

Los cinco checks de CI están verdes. Sigue pendiente el smoke real en GNOME/Mutter con mapeo lento, tal como se declara en la PR; no ejecuté pruebas locales ni ese smoke en esta revisión.

Por mi parte, recomiendo aprobar. La aprobación formal y el merge quedan a cargo del mantenedor.

@egdev6
egdev6 merged commit 83b0792 into egdev6:main Oct 31, 2026
5 checks passed
egdev6 added a commit that referenced this pull request Oct 31, 2026
…and (#215) (#224)

* fix(linux/x11): ajustar la ventana del detail al contenido bajo XWayland (#215)

La ventana detail nace resizable(false) y GTK retiene un piso PMinSize sticky,
así que el resize externo es rechazado en el ancho y el aviso de API key muestra
la caja del contenedor (~16 px de banda). El detalle pasa por el mismo ciclo X11
de la tira (#205): una sola llamada al hilo de UI hace la pista de tamaño, las
pistas min == max == pedido, el permiso de resize abierto sólo alrededor del
resize y confirmado cerrado; luego la posición explícita y un read-back de GDK
con presupuesto acotado. Generación de cancelación propia del detail (no
comparte la del widget), sin anclaje y con informe {status, bounds, reason,
read_back} espejo de set_widget_bounds; Windows, macOS y Wayland nativo quedan
en su ruta anterior (unverified, bounds=null).

* fix(linux/x11): el panel del detail consume el informe de colocación (#215)

DetallePanel cachea sólo la geometría confirmada por read-back (nunca la pedida),
repide en silencio tras show() ante not_viewable con el presupuesto acotado de
#221 (readPlacementWithRetries reutilizado) y deja intacto el flujo de Wayland
nativo/Windows/macOS (unverified sin read-back, comparación exacta). La apertura
no se bloquea: place → clip → positioned → show conserva su orden.

* docs(i18n): regenerar el inventario de textos (#215)

Secciones nuevas de detail_placement.rs y detail_placement_test.rs (cuerpos de
error y nombres de test, nada de interfaz de usuario) tras los commits 9a6622d
y 15aea55; el frontend no añade textos.

* test(linux/x11): pedir posición dentro del área en la prueba en vivo (#215)

Ejecutar la prueba en vivo contra el XWayland real marcó el hallazgo del
fixture: movía la ventana 314×379 a (1859,899), fuera de pantalla, y el gestor
la reubicaba (1606,701) para que quepa. La petición ahora es (100,100) y el
resto de la secuencia queda intacto: tamaño exacto concedido, permiso cerrado y
tamaño sobreviviente a la retirada.

* docs(odd): artefacto de la colocación nativa del detail en X11 (#215)

Ciclo, decisiones (D1–D3), TDD por unidad, hallazgo del fixture de la prueba
en vivo y evidencia de sesión GNOME/XWayland (geometría 314×379, pistas
min == max, sin caja translúcica) según los criterios de aceptación (#215).

* docs(i18n): regenerar el inventario de textos

* fix(linux/x11): mover el detail a coordenadas lógicas en HiDPI (#215)

request_position pasaba la petición física tal cual a gtk.move_, que
mide en píxeles lógicos: a escala ×2 el panel quedaba desplazado
aproximadamente al doble (P2 del review de PR #224, diagnosticado por
el mantenedor y contrastado con el recorrido del código).

Se introduce el seam testeable del propio archivo (Positioner +
GtkPositioner + position_call, espejo de Sizable/GtkSizable): la
conversión con to_logical cubre los dos ejes antes del movimiento, el
mismo mapeo que el adaptador del widget. La cobertura nueva (escala
×2: (200,300) físico -> (100,150) lógico) falla contra el código
anterior, así que la omisión queda atrapada por test.

Verificación: cargo test 402 + 4 ignoradas (0 fallos), fmt limpio,
clippy sin avisos nuevos en detail_placement*; verificador
independiente en PASS sobre los 5 checks.

* docs(odd): bitácora L5 del fix HiDPI del review de PR #224 (#215)

* docs(i18n): regenerar el inventario de textos (#215)

* docs(odd): cierre L5 con el inventario i18n regenerado (#215)

---------

Co-authored-by: Quique <egdev6@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants