chore(recorder): unify double click handling, drop action timestamps#41732
Open
dgozman wants to merge 4 commits into
Open
chore(recorder): unify double click handling, drop action timestamps#41732dgozman wants to merge 4 commits into
dgozman wants to merge 4 commits into
Conversation
Remove `FrameDescription.framePath` and instead prepend the chained frame selectors, separated by `internal:control=enter-frame` tokens, straight into `action.selector`. `asLocator()` already renders that token as `contentFrame()`, so the four language generators no longer need to build the frame chain by hand. Also fixes `JsonlLocatorFactory.chainLocators()` clobbering an existing `next` chain, which dropped the `frame` step from JSONL output.
Remove `ActionBase.preconditionSelector`. The injected recorder now passes the precondition selector to the server as a separate binding argument, and the server turns it into an `ExpectSignal` routed through the signal processor, so it attaches to the preceding action like any other signal. Each language generator renders the signal itself, appending an `assertVisible` after the action it is attached to. `generateAction()` now receives `LanguageGeneratorOptions` so generators can honour `generateExpectSignal`, renamed from `generateAutoExpect`.
The recorder was naming the pages of the generated source code (page, page1, ...) and stamping the name onto every action. Drop it: actions and signals now carry only `pageGuid`, and each language generator lazily mints its own aliases into a pageGuid -> alias map, cleared by a new `reset()` that `generateCode()` calls. `PopupSignal.popupAlias` was the popup page's own alias, so it becomes `popupPageGuid` and resolves through the same map. `FrameDescription` held nothing else, so `pageGuid` moves onto `ActionInContext`/`SignalInContext` directly, and `mainFrameForAction()` goes away — the caller already has the frame the action came from.
JsonRecordActionTool recorded every click as it arrived, so a double click produced two actions that shouldMergeAction() had to collapse. Give it the same stalled-click handling RecordActionTool has, and drop the click branch of shouldMergeAction(). While stalling a click, a following click cancelled the pending one instead of committing it, silently swallowing the first click. Both tools now commit the pending click when a new single click arrives, and only cancel it when the click continues a multi-click, which 'dblclick' reports instead. Also remove ActionInContext.startTime/endTime and SignalInContext.timestamp. endTime and timestamp were never read; startTime had a single consumer, the navigation signal threshold, which now lives in RecorderSignalProcessor._lastActionTimestamp.
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.
Unified double click handling
RecordActionToolstalls a single click for 200ms and reports a double click fromdblclick, so aclickCount: 1action never reaches the server.JsonRecordActionTool(used byrecorderMode: 'api') had no such handling — it recorded every click as it arrived, so a double click produced two actions and relied on theclickbranch ofshouldMergeAction()to collapse them into one.The Json tool now stalls clicks the same way, and that branch of
shouldMergeAction()is gone.Two consequences:
recorder-api.spec.tsassertions becomeexpect.poll.clickCount: 2rather than3, matching what the regular recorder has always produced.Pending click is no longer swallowed
Both tools opened
onClickwith an unconditional_cancelPendingClickAction(), which discarded a stalled click whenever another click arrived. Two clicks less than 200ms apart, even on different elements, silently lost the first one. Rare for a human, but routine forpage.click()driving the API recorder.Both tools now commit the pending click when the new click is
detail === 1(a genuinely new click), and only cancel it whendetail > 1(part of a multi-click, whichdblclickreports). The commit/cancel also moved above the checkbox branch, so a stalled click followed by a checkbox click no longer records the two out of order.This lets
should record expect signaldrop the poll it used to need to avoid losing its first click.Timestamps removed
No client reads them:
ActionInContext.endTime— written only, never read.SignalInContext.timestamp— written only, never read.ActionInContext.startTime— one consumer, the navigation-signal threshold, nowRecorderSignalProcessor._lastActionTimestamp.Recorderno longer importsmonotonicTimeat all.