Fixed a bug with removed or replaced form fields on some websites.
This commit is contained in:
1 parent
919b9e013b
commit
9e0ff41b16
6 files changed
+191
-7
No files matched your search
@@ -287,7 +287,7 @@ func (adapter *Adapter) readNativeDocumentCaret(ctx context.Context, position Ob
|
||||
var nativeErr *C.GError
|
||||
count := int(C.atspi_text_get_character_count(text, &nativeErr))
|
||||
if nativeErr != nil {
|
||||
err := nativeError("get document text count", nativeErr)
|
||||
err := adapter.document_object_error(position.ID(), nativeError("get document text count", nativeErr))
|
||||
C.g_error_free(nativeErr)
|
||||
return DocumentCaretText{}, err
|
||||
}
|
||||
@@ -340,7 +340,7 @@ func (adapter *Adapter) readNativeDocumentCaret(ctx context.Context, position Ob
|
||||
}
|
||||
rangeText := C.atspi_text_get_string_at_offset(text, C.gint(offset), granularity, &nativeErr)
|
||||
if nativeErr != nil {
|
||||
err := nativeError("read document text", nativeErr)
|
||||
err := adapter.document_object_error(position.ID(), nativeError("read document text", nativeErr))
|
||||
C.g_error_free(nativeErr)
|
||||
return DocumentCaretText{}, err
|
||||
}
|
||||
@@ -494,7 +494,7 @@ func (adapter *Adapter) setNativeDocumentPosition(ctx context.Context, position
|
||||
} else {
|
||||
caretOffset = C.atspi_text_get_caret_offset(text, &nativeErr)
|
||||
if nativeErr != nil {
|
||||
err := nativeError("get document caret", nativeErr)
|
||||
err := adapter.document_object_error(position.ID(), nativeError("get document caret", nativeErr))
|
||||
C.g_error_free(nativeErr)
|
||||
clear(adapter.navigationEcho)
|
||||
return err
|
||||
@@ -522,7 +522,7 @@ func (adapter *Adapter) setNativeDocumentPosition(ctx context.Context, position
|
||||
}
|
||||
moved := C.atspi_text_set_caret_offset(text, caretOffset, &nativeErr)
|
||||
if nativeErr != nil {
|
||||
err := nativeError("set document caret", nativeErr)
|
||||
err := adapter.document_object_error(position.ID(), nativeError("set document caret", nativeErr))
|
||||
C.g_error_free(nativeErr)
|
||||
clear(adapter.navigationEcho)
|
||||
return err
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
// Copyright (c) 2026 Storm Dragon
|
||||
// Authors: Storm Dragon, Angus Kola
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package atspi
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// libatspi forwards Chromium's missing-object response as an untyped GError.
|
||||
// Match the queried path exactly; a timeout or unsupported caret is not proof
|
||||
// that the virtual reading position has disappeared.
|
||||
func (adapter *Adapter) document_object_error(object ObjectID, err error) error {
|
||||
if err == nil || !object.Valid() || !strings.HasSuffix(strings.TrimSpace(err.Error()),
|
||||
"could not be processed as object "+object.Path+" does not exist") {
|
||||
return err
|
||||
}
|
||||
adapter.treeMutex.Lock()
|
||||
trees := []*AccessibleTree{adapter.tree, adapter.documentTree, adapter.documentTextTree}
|
||||
adapter.treeMutex.Unlock()
|
||||
for _, tree := range trees {
|
||||
if tree == nil {
|
||||
continue
|
||||
}
|
||||
tree.mu.RLock()
|
||||
entry := tree.nodes[object]
|
||||
tree.mu.RUnlock()
|
||||
adapter.treeInvalidations.Add(uint64(tree.Invalidate(object)))
|
||||
if entry.hasParent {
|
||||
// The parent's cached child list may also predate the removal.
|
||||
// Refresh it to discover any replacement while retaining siblings.
|
||||
tree.MarkChildrenChanged(entry.parent)
|
||||
}
|
||||
}
|
||||
clear(adapter.documentLineCache)
|
||||
return fmt.Errorf("%w: %w", ErrDeadObject, err)
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
// Copyright (c) 2026 Storm Dragon
|
||||
// Authors: Storm Dragon, Angus Kola
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package atspi
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestDocumentMissingObjectRetiresCachedTarget(t *testing.T) {
|
||||
adapter := New()
|
||||
adapter.tree, adapter.documentTree, adapter.documentTextTree = testTree(t), testTree(t), testTree(t)
|
||||
object := testID("/second")
|
||||
failure := fmt.Errorf("get document caret: Method %q with signature %q on interface %q could not be processed as object %s does not exist\n",
|
||||
"Get", "ss", "org.freedesktop.DBus.Properties", object.Path)
|
||||
err := adapter.document_object_error(object, failure)
|
||||
if !errors.Is(err, ErrDeadObject) || !errors.Is(err, failure) {
|
||||
t.Fatalf("missing-object error lost classification or details: %v", err)
|
||||
}
|
||||
for _, tree := range []*AccessibleTree{adapter.tree, adapter.documentTree, adapter.documentTextTree} {
|
||||
if _, err := tree.Reference(object); !errors.Is(err, ErrObjectNotFound) {
|
||||
t.Fatalf("removed target remains cached: %v", err)
|
||||
}
|
||||
if _, err := tree.Reference(testID("/grandchild")); !errors.Is(err, ErrObjectNotFound) {
|
||||
t.Fatalf("removed descendant remains cached: %v", err)
|
||||
}
|
||||
if _, err := tree.Reference(testID("/first")); err != nil {
|
||||
t.Fatalf("live sibling was retired: %v", err)
|
||||
}
|
||||
root, _ := tree.Root()
|
||||
if _, _, err := tree.FirstChild(root); !errors.Is(err, ErrIncompleteTree) {
|
||||
t.Fatalf("parent child order was not marked for refresh: %v", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestDocumentCaretFailuresDoNotRetireLiveTargets(t *testing.T) {
|
||||
for _, message := range []string{
|
||||
"browser refused document caret",
|
||||
"get document caret: timeout",
|
||||
"get document caret: unknown property CaretOffset",
|
||||
"could not be processed as object /other does not exist",
|
||||
"could not be processed as object /second_child does not exist",
|
||||
} {
|
||||
t.Run(message, func(t *testing.T) {
|
||||
adapter := New()
|
||||
adapter.documentTree = testTree(t)
|
||||
failure := errors.New(message)
|
||||
if err := adapter.document_object_error(testID("/second"), failure); err != failure {
|
||||
t.Fatalf("ordinary failure reclassified: %v", err)
|
||||
}
|
||||
if _, err := adapter.documentTree.Reference(testID("/second")); err != nil {
|
||||
t.Fatalf("unconfirmed removal retired live target: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -157,6 +157,12 @@ func (worker *Worker) placeDocumentWithRetry(ctx context.Context, cursor *Docume
|
||||
return worker.placeDocument(ctx, cursor, result)
|
||||
}
|
||||
result = place(result)
|
||||
if errors.Is(result.CaretErr, atspi.ErrDeadObject) {
|
||||
// A native caret refusal can leave a valid virtual position. A provider
|
||||
// confirming that the target no longer exists cannot: do not announce
|
||||
// its cached label as a successful move, even if recapture fails.
|
||||
result.Err, result.Moved = result.CaretErr, false
|
||||
}
|
||||
if !documentCaretNeedsRecapture(result.CaretErr) || !worker.CurrentDocumentResult(result) {
|
||||
return result
|
||||
}
|
||||
@@ -188,8 +194,12 @@ func (worker *Worker) placeDocumentWithRetry(ctx context.Context, cursor *Docume
|
||||
result.DocumentPosition.Object = target
|
||||
result.DocumentPosition.Document = owner.TopLevel
|
||||
result.DocumentPosition.Snapshot = snapshot
|
||||
result.Err, result.Moved = nil, true
|
||||
result.CaretErr = nil
|
||||
result = place(result)
|
||||
if errors.Is(result.CaretErr, atspi.ErrDeadObject) {
|
||||
result.Err, result.Moved = result.CaretErr, false
|
||||
}
|
||||
if result.Err == nil && result.CaretErr == nil && cursor != nil {
|
||||
cursor.rebindPlace(tree, oldPosition, result.DocumentPosition)
|
||||
}
|
||||
@@ -198,7 +208,7 @@ func (worker *Worker) placeDocumentWithRetry(ctx context.Context, cursor *Docume
|
||||
|
||||
func documentCaretNeedsRecapture(err error) bool {
|
||||
return errors.Is(err, atspi.ErrObjectNotFound) || errors.Is(err, atspi.ErrStaleObject) ||
|
||||
errors.Is(err, atspi.ErrStaleSnapshot)
|
||||
errors.Is(err, atspi.ErrStaleSnapshot) || errors.Is(err, atspi.ErrDeadObject)
|
||||
}
|
||||
|
||||
func (worker *Worker) moveDocumentCaret(ctx context.Context, cursor *DocumentCursor, command DocumentCaretCommand, epoch uint64) (Result, *DocumentCursor) {
|
||||
@@ -477,7 +487,7 @@ func (worker *Worker) recoverCapturedDocumentFocus(cursor *DocumentCursor, tree
|
||||
|
||||
func documentNeedsCapture(err error) bool {
|
||||
return errors.Is(err, ErrNoCurrent) || errors.Is(err, atspi.ErrObjectNotFound) ||
|
||||
errors.Is(err, atspi.ErrStaleObject) || errors.Is(err, atspi.ErrIncompleteTree) ||
|
||||
errors.Is(err, atspi.ErrStaleObject) || errors.Is(err, atspi.ErrDeadObject) || errors.Is(err, atspi.ErrIncompleteTree) ||
|
||||
errors.Is(err, atspi.ErrStaleText) || errors.Is(err, atspi.ErrStaleGeometry) || errors.Is(err, atspi.ErrStaleSnapshot)
|
||||
}
|
||||
|
||||
|
||||
@@ -643,7 +643,7 @@ func (navigator *fakeDocumentNavigator) SetDocumentPosition(ctx context.Context,
|
||||
}
|
||||
|
||||
func TestWorkerRetriesStaleDocumentCaretOnSameTarget(t *testing.T) {
|
||||
for _, failure := range []error{atspi.ErrStaleObject, atspi.ErrObjectNotFound} {
|
||||
for _, failure := range []error{atspi.ErrStaleObject, atspi.ErrObjectNotFound, atspi.ErrDeadObject} {
|
||||
t.Run(failure.Error(), func(t *testing.T) {
|
||||
tree, _ := webCursor(t)
|
||||
navigator := newFakeDocumentNavigator()
|
||||
@@ -694,6 +694,74 @@ func TestWorkerRetriesStaleDocumentCaretOnSameTarget(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkerRemovedEntryIsNotPresentedAndNextCommandRecovers(t *testing.T) {
|
||||
tree, _ := webCursor(t)
|
||||
nodes := webNodes()
|
||||
for index := range nodes {
|
||||
if nodes[index].Snapshot.Path == "/first" || nodes[index].Snapshot.Path == "/second" {
|
||||
nodes[index].Snapshot.Role = "combo box"
|
||||
nodes[index].Snapshot.Editable = true
|
||||
nodes[index].Snapshot.Focusable = true
|
||||
}
|
||||
}
|
||||
if err := tree.Replace(webID("/root"), nodes); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
navigator := newFakeDocumentNavigator()
|
||||
worker, cancel := startWorker(t, navigator, time.Second)
|
||||
defer worker.Stop(context.Background())
|
||||
defer cancel()
|
||||
worker.SyncFocus(webID("/button"))
|
||||
navigator.placementErr <- atspi.ErrDeadObject
|
||||
if err := worker.MoveDocument(DocumentEntry, 0, true); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
initial := <-navigator.fakeCapturer.calls
|
||||
initial.result <- captureResult{tree: tree}
|
||||
first := <-navigator.calls
|
||||
if first.position.ID() != webID("/second") {
|
||||
t.Fatalf("initial entry target = %+v", first)
|
||||
}
|
||||
retry := <-navigator.fakeCapturer.calls
|
||||
// The native adapter retires a confirmed missing target before recapture.
|
||||
tree.Invalidate(webID("/second"))
|
||||
retry.result <- captureResult{tree: tree}
|
||||
result := waitResult(t, worker)
|
||||
if result.Moved || !errors.Is(result.Err, atspi.ErrDeadObject) {
|
||||
t.Fatalf("removed entry reported as a successful move: %+v", result)
|
||||
}
|
||||
if err := worker.MoveDocument(DocumentEntry, 0, true); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
placement := <-navigator.calls
|
||||
if placement.position.ID() != webID("/first") {
|
||||
t.Fatalf("next command revisited removed entry: %+v", placement)
|
||||
}
|
||||
next := waitResult(t, worker)
|
||||
if next.Err != nil || next.CaretErr != nil || !next.Moved {
|
||||
t.Fatalf("next entry navigation failed: %+v", next)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkerCaretRefusalKeepsVirtualMove(t *testing.T) {
|
||||
_, cursor := webCursor(t)
|
||||
navigator := newFakeDocumentNavigator()
|
||||
worker, err := NewWorker(navigator, time.Second)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
worker.SyncFocus(webID("/first"))
|
||||
failure := errors.New("browser refused document caret")
|
||||
navigator.placementErr <- failure
|
||||
result, _ := worker.moveDocument(context.Background(), cursor, DocumentHeading, 0, false, worker.DocumentEpoch())
|
||||
if result.Err != nil || result.CaretErr != failure || !result.Moved || result.DocumentPosition.Object.ID() != webID("/second") {
|
||||
t.Fatalf("ordinary caret refusal lost virtual reading position: %+v", result)
|
||||
}
|
||||
if len(navigator.fakeCapturer.calls) != 0 {
|
||||
t.Fatal("ordinary caret refusal unnecessarily recaptured the document")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkerDocumentNavigation(t *testing.T) {
|
||||
tree, _ := webCursor(t)
|
||||
capturer := &fakeCapturer{calls: make(chan captureCall, 2)}
|
||||
|
||||
Reference in new issue
Block a user