1887093 - Text selection cleared when screenshot is taken

Status: VERIFIED FIXED

(Firefox :: Screenshots, defect, P1)

Description

[Andrew Overholt [:overholt]]

STR

1. go to a website and select some text
2. attempt to take a screenshot

Expected

• text remains highlighted in the screenshot

Actual

• selection goes away

Video demonstrating this: https://drive.google.com/file/d/1JlBbQfzsSkk0YvYb4nNxCOO_1M7WCzgF/view?usp=sharing

macOS 14.4
Firefox Nightly (2024-03-22)

Comment 1

[Sam Foster [:sfoster] (he/him)]

I think this is because we're blur-ing the input when the screenshots UI is opened. :emilio do you know if there's anything we can do to avoid this? The selection is still there, but when the input loses focus we don't paint it. However, if you hit F6 we do show both the content focus and chrome focus.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Well, the screenshots UI is part of the content, so we don't track focus there separately... Maybe focus shifts shouldn't clear the selection? That'd at least fix the regular text caret... Masayuki, thoughts?

Comment 3

[Sam Foster [:sfoster] (he/him)]

The selection still exists. At least, if you re-focus the field it is correctly visibly restored. But we don't visibly show it on a blur'd field. Or is it technically cleared but tracked in-memory so we can restore it on focus?

As you point out - some of the Screenshots UI lives in the content so maybe this is expected behavior as we do want to steal focus allowing users to navigate that overlay UI from the keyboard. Our other option would be to try to restore focus to the content document right before we actually capture the screenshot and put it back into the Screenshots UI directly after. That might be a bit "noisy" for screen readers though.

Comment 4

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot 2024-03-29 at 17-07-50 1887093 - Text selection cleared when screenshot is taken.png

Actually, it looks like focus might be a red herring here. When I took this screenshot I had selected the first paragraph of my previous comment (i.e. a text selection not in a editable form field.) Selecting (highlighting) text and capturing that in a screenshot seems like a reasonable user expectation.
(Correction: the text selection is visible beneath the screenshots overlay only until you move focus using tab or F6, then selection disappears)

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

As far as I checked around Selection, selection is cleared by the screenshot UI button gets focus caused by a call of Element.focus(). Then, nsFocusManager::Focus() calls UpdateCaret(), then, UpdateCaret() calls MoveCaretToFocus(), finally, MoveCaretToFocus() calls Selection::RemoveAllRanges().

I wonder, FocusOptions could have a chrome only option to prevent selection move. Then, nsFocusManager::ProgrammaticFocusFlags() should have same option, finally, the UpdateCaret() caller should set the first parameter to false if the new flag is set.

However, I'm not sure whether this is the only cause of selection callers. If there are too many cases, nsIForcusManager should have a mode to avoid moving Selection, but it's riskier than the above approach.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I investigate the design of nsFrameSelection more, then, it seems that making the shadow tree for the screenshot UI have an independent selection is the ideal fix because web apps should never see odd selection ranges computed from native anonymous subtree. However, I think that it requires a lot of changes around nsFrameSelection.

My current question is, "Do we really need to move selection when a <button> gets focus?" If it's false from a11y point of view, I think that nsFocusManager::UpdateCaret() or MoveCaretToFocus() should not touch Selection when new focused element is in a native anonymous subtree, but it's not an independent selection for a text control.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

My current question is, "Do we really need to move selection when a <button> gets focus?"

My understanding is that we need to because the caret is also the focus navigation starting point, and the find-in-page start point, generally... Though that indeed doesn't apply to this particular case.

making the shadow tree for the screenshot UI have an independent selection is the ideal fix

I mean, that might be worth it, and it shouldn't be too bad perhaps? We could have a CSS property of sorts and add it here or what not?

Comment 8

[Niklas Baumgardner [:niklas]]

Attachment: Bug 1887093 - Remember text selection regions during screenshots interactions. r=sfoster

Comment 9

[Niklas Baumgardner [:niklas]]

I've just posted a patch for this that handles this by remembering the selected regions before screenshots was opened and re-adds the regions when we get a selection change event from focusing an element in the screenshots overlay.

Unless an independent selection for the anonymous content is a high priority, I don't think we need it for this bug.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:sfoster, could you have a look please?

For more information, please visit BugBot documentation.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d87d3d78ebe4
Remember text selection regions during screenshots interactions. r=sfoster

Comment 12

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/d87d3d78ebe4

Comment 14

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on 2024-04-15 Nightly build on macOS 12.
Verified as fixed on Firefox Nightly 128.0a1 and Firefox 127.0b2 on macOS 12 and Ubuntu 22.
On Windows 10 I can still reproduce this issue in Firefox Nightly 128.0a1, but not Firefox 127.0b2.
Should i file a bug for Windows 10 only?
Thank you.

Comment 15

[Niklas Baumgardner [:niklas]]

(In reply to Ardelean Oana, Desktop QA [:oardelean] from comment #14)

On Windows 10 I can still reproduce this issue in Firefox Nightly 128.0a1, but not Firefox 127.0b2.
Should i file a bug for Windows 10 only?
Thank you.

Yes, please! That would be helpful

Comment 16

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

I filled bug 1897371 for Windows.
Updating the status flags based on Comment 14.