Text Selection popover doesn't run action in Gmail email composer

[twschiller]

Describe the bug

Text selection popover over buttons don't work in Gmail email composer

To Reproduce

• Enable the text selection popover
• Add a context menu for selection (e.g., one that shows the selected text as an alert)
• Open the email composer in Gmail
• Write "hello", select the text, and click your action

Actual behavior

• Popover disappears but doesn't run

Expected behavior

• Action runs and popover disappears

Investigation

• What appears to happening is that upon clicking the the popover, the selectionchange event fires with an empty selection, so tooltipController calls hideTooltip. It likely calls the Abort before the click has a chance to run
• If you remove the hideTooltip() in the selectionchange handler, the action runs, but doesn't see any selected text

Desktop (please complete the following information):

• Text Command Popover Fixes/UI Improvements #7728

[fregante]

Findings:

1. a plain document.execCommand('insertText', false, 'hello') works from the console

2. I can repro this issue from the MV2 sidebar too.

3. document.activeElement switches to the sidebar iframe, but document.execCommand() still works correctly on the playground, while it doesn't on Gmail

4. clicking on an element in the same page changes the selection and focus on mousedown unless the element is an actual button 🤯

   

So:

• perhaps we need to track the change of focus on all documents so that if document.activeElement === our iframe, we can restore it

[fregante]

A couple more demos showing the regular behavior, clicking and dragging:

and the broken behavior on Gmail, where it disappears on mousedown:

Gmail is breaking the native behavior by manually deselecting the text on mousedown:

and it ends up calling either one of these:

pixiebrix-extension/src/contentScript/selectionTooltip/tooltipController.tsx

Line 291 in 4878e51

hideTooltip();

pixiebrix-extension/src/contentScript/selectionTooltip/tooltipController.tsx

Lines 86 to 94 in 4878e51

	"selectstart",
	// Avoid sticky tool-tip on SPA navigation
	"navigate",
	]) {
	document.addEventListener(documentEventType, hideTooltip, {
	passive: true,
	once: true,
	signal: hideController.signal,
	});

Side note: The logic in these two should be combined.

I'm unsure on how to proceed because it's hard to determine whether the selectionchange was intentional by the user or if caused by Gmail. This matters because it's when we have to decide whether to use the "previous" selection or the direct result of window.getSelection()

Maybe the solution is to use selectionController.save() when the user hovers our popover.

[twschiller]

Reopening because it's still not generally working on 1.8.10-beta.2-mv3 on Chrome: https://www.loom.com/share/1934bedba08f43dd88066fd2fea11f25?sid=5cb7826f-d245-4264-8dc7-a91e43f93602

@fregante should I give MV2 a try?

@grahamlangford I don't think this is a blocker for 1.8.10

[twschiller]

Side note: The logic in these two should be combined.

Which two are you referring to? The React Component and tooltipController? Or the listeners in showTooltip and initSelectionTooltip?

For me, splitting the React Component and Controller isn't essential, the crux of the issue is still deciding:

• Under what circumstances to hide the popover
• Which selection should be used when the popover fires: 1) the selection at the time the popover was shown, or 2) the current selection (which might have been programmatically selected)

There's likely a gotcha around useDocumentSelection. We should probably be using the selection at the time the popover was shown vs. at the time the button was clicked

I'm unsure on how to proceed because it's hard to determine whether the selectionchange was intentional by the user or if caused by Gmail.

A couple of options are:

• Look at https://developer.mozilla.org/en-US/docs/Web/API/Event/isTrusted to determine if the user initiated the change. Update: somehow from Gmail composer event is isTrusted, so this won't work for that case
• Keep track of whether there was a selectionstart within a certain amount of time before the selectionchange

Going to try some changes. Removing the hide on selectionchange wouldn't be the end of the world

[fregante]

Side note: The logic in these two should be combined.

Which two are you referring to?

I was referring to the two pieces of code I linked before the note, and then also merge the logic into the React component. I think it's easier to reason about it when the logic is in a single place. Also delays might be easier to add in the React component without risking that they get out of sync (e.g. a hide command is scheduled via setTimeout, but another show event happens before it fires)