1715603 - Shift+Middle-click select/highlights page content

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P3)

Description

[anthony.lizot]

Attachment: Shift+Middle-clic on Google Search.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

When browsing on any page, when left-clicking somewhere in the page and then Shift+Middle-clicking anywhere else, on a link to open it in a new background (or foreground, depending on user preferences) tab for example, the text between the two locations is selected/highlighted.

Actual results:

The text between the two locations is selected/highlighted, although it should not.

Expected results:

Only Shift+Left-clicking should select/highlight the text between the two locations.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox::Preferences' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Peter Magyari (Desktop QA)]

Hi,

I was able to reproduce this issue on Firefox 89.0 and Firefox Nightly 90.0a1

I will look for a regression range asap.

Thank you for reporting!

Comment 3

[Peter Magyari (Desktop QA)]

Mozregression: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2032f2b926ddf13702d5720983d226273a4290a6&tochange=889132330748b03914570e29abbb56950816da30

I believe Bug 1528289 has introduced this issue.

Comment 4

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

I'll take a look on next week, but I'm away until Wed, so, feel free to take this if you want to fix this ASAP.

Comment 6

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

Hmm, Chrome and Safari do the new behavior of us. So, keeping this change must be safer for web-compat. (And also Chrome and Safari extend selection even with Right click. So, we may need to change right click behavior too if we keep this behavior.)

WDYT, smaug, edgar?

(FYI: I don't mind to create a new pref to keep the traditional behavior for the our users who don't like this behavior.)

Comment 7

[Alice0775 White]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #6)

Hmm, Chrome and Safari do the new behavior of us. So, keeping this change must be safer for web-compat.

I do not think so. I can not reproduce the issue on Chrome91 Windows10.

Simple steps to reproduce:
0. New profile

1. Open web page in [Tab A] (e.g. https://ftp.mozilla.org/pub/firefox/nightly/)
2. Middle-click on a link(e.g. 2004) so that the link opens in a new background tab
3. Shift + Middle-click on the other link(e.g. 2010) so that the link opens in a new foreground tab
4. Back to previous tab[Tab A]

Actual Results:
Text is unexpectedly selected. -- BUG!!
Chrome does not select any text!! --OK

Expected Results:
Text should not be selected.

Comment 8

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

(In reply to Alice0775 White from comment #7)

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #6)
Simple steps to reproduce:
0. New profile

1. Open web page in [Tab A] (e.g. https://ftp.mozilla.org/pub/firefox/nightly/)
2. Middle-click on a link(e.g. 2004) so that the link opens in a new background tab
3. Shift + Middle-click on the other link(e.g. 2010) so that the link opens in a new foreground tab
4. Back to previous tab[Tab A]

Actual Results:
Text is unexpectedly selected. -- BUG!!
Chrome does not select any text!! --OK

Expected Results:
Text should not be selected.

Interesting. It's an edge case of this bug. Chrome does not modify selection range with clicking in a link. Perhaps, it consumes the event internally. This should be fixed, but I'm still not sure about clicking in normal text nodes.

Comment 9

[Martin]

Just curious, why is this being marked as wontfix? This weird behavior is not seen in Chrome, Microsoft Edge, Safari, or any other browser that I know of. It's obviously a regression and has not been a problem in any previous version of Firefox I have ever used (from version 4 to current version) as far as I remember.

Comment 10

[anthony.lizot]

I think it's too late to fix this issue for the current (89) and the coming one (90), that's why it's marked as "won't fix" for both. So I think we can expect it will be fixed in the next releases (91+). It's just my opinion.

Comment 11

[Pascal Chevrel:pascalc]

(In reply to anthony.lizot from comment #10)

I think it's too late to fix this issue for the current (89) and the coming one (90), that's why it's marked as "won't fix" for both. So I think we can expect it will be fixed in the next releases (91+). It's just my opinion.

This is correct, 89 is already out and 90 had its last beta yesterday.

Comment 12

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

Okay, I think that we should do:

• Stop extending selection to a link for compatibility with Chrome and avoiding double default actions
• Collapse selection into a link in contenteditable for compatibility with Chrome
• Stop starting autoscroll if a modifier key is pressed for compatibility with Chrome and avoiding double default actions

but do not:

• Stop extending selection in the other cases for compatibility with Chrome

Comment 13

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

Attachment: Bug 1715603 - part 1: Don't extend selection into a link r=edgar!

If middle button click with Shift key occurs, Chrome and Safari extend the
selection in most cases. However, if the clicked position is in a link,
Chrome does:

• If editable, collapse selection into the link instead of extending selection.
• If not editable, not extending selection and open tabs.

We should follow this behavior for both backward compatibility and web-compat.

Comment 14

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

Attachment: Bug 1715603 - part 2: Make `AutoScrollChild` not start autoscroll if a modifier key is pressed r=Gijs!

Now, nsIFrame::HandleEvent moves selection at middle mouse button down. This
occurs before dispatching the event into the system event group. Therefore,
AutoScrollChild cannot prevent it.

On the other hand, Chrome does not start autoscroll when a modifier is pressed.
This means that our users may not be able to use middle click with modifiers
if web apps do not call preventDefault() as expected. So, this difference
is a potential risk of web-compat.

Therefore, this patch makes AutoScrollChild stop starting autoscroll if
a modifier key is pressed.

Depends on D119252

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3220a2731b3
part 1: Don't extend selection into a link r=edgar
https://hg.mozilla.org/integration/autoland/rev/8cfe1acc1580
part 2: Make `AutoScrollChild` not start autoscroll if a modifier key is pressed r=Gijs

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29647 for changes under testing/web-platform/tests

Comment 17

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d3220a2731b3
https://hg.mozilla.org/mozilla-central/rev/8cfe1acc1580

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 19

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

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 20

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

I think that patch is risky to uplift to beta because if some environments emulates middle mouse button with Shift + something, the user cannot use autoscroll without changing the new hidden pref. So, we should wait to reply from Nightly and Beta testers about this. Then, if no regression reports, we should uplift this to ESR 91 since this is not a minor regression.

Comment 21

[Peter Magyari (Desktop QA)]

I've verified the fix on Firefox 92.0a1 (20210719215029)

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Did you want to nominate this for ESR91 uplift?

Comment 23

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

Comment on attachment 9230001 [details]
Bug 1715603 - part 1: Don't extend selection into a link r=edgar!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a regression of major feature of middle mouse click.
• User impact if declined: When user tries to open a link with middle mouse click, user lost their own selected range. This is different behavior from the other browsers (the original fix was for web-compat issue of middle mouse click behavior).
• Fix Landed on Version: 92
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Partially stopped running new path which regressed.

And the following patch disables that Shift + Middle mouse click does not change selection too. However, if user emulates middle click with Shift key, they need to disable this patch's behavior with the new pref. This feature is separated to the following patch. So, if ESR shouldn't take any behavior change, only uplifting part.1 is also okay. But I recommend to uplift both for consistent behavior with 92 and later releases.

• String or UUID changes made by this patch: none

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9230001 [details]
Bug 1715603 - part 1: Don't extend selection into a link r=edgar!

Given how early in the ESR91 lifecycle we are, let's take both patches for consistency with release. Approved for 91.1esr.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/39537a4196d4
https://hg.mozilla.org/releases/mozilla-esr91/rev/48135b8a330e