1754459 - Images get highlighed when clicked

Status: VERIFIED FIXED

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

Description

[Marco]

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

Steps to reproduce:

Open https://www.peopleinside.it/
Click on images they will selected.

See: https://github.com/webcompat/web-bugs/issues/99347

Actual results:

Images are showed selected

Expected results:

Image should be not showed selected

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=71d18b1a1fc12b44eac1fba2afa0a28ba5231ad3&tochange=e6f994394be4fd1eb00bd8db40e8a8dcb42f5f9a

Comment 2

[Dennis Schubert [:denschub]]

The patch that regresses this says

This patch restores the behavior to select a non-draggable & non-editable image to it was before bug 1550869. That is, we recognize the image selection when the mouse pointer moves passed the middle point of the image width (OffsetsForSingleFrame).

and based on that, I don't think that just clicking an image should be triggering this.

Ting-Yu, could you please look at this case and see what we can do about this? :)

Comment 3

[Dennis Schubert [:denschub]]

[Tracking Requested - why for this release]:

This is a regression in 97, where just clicking an image select-highlights the entire image. This isn't breaking anything, but probably can be considered a minor visual annoyance. Requesting tracking just so this is on your radar.

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

The images has pointer-events: none that seems to trigger this annoyance. After changing it to default pointer-events: auto, it behaves as expected. I haven't looked into how that can trigger the selection for the image. (keeping the NI as a reminder.)

Comment 5

[Marco]

Thank you!
For avoid this bad Firefox visual in my website I will add the following CSS:

user-select: none;

You should still be able to deactivate this in the browser console and still continue to diagnose and fix this visual issue on Firefox.
For anything I will be here, following this issue until the end.

If you never see "the bug" is because I added

user-select: none;

as temporary fix.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Unlikely we'll do anything to address this out-of-band in Fx97 at this point unless we get more reports of this being more widespread, but let's keep it on the radar for 98 at least.

Comment 7

[Marco]

Oh, "nice" to see will be not fixed even in version 98 ^_^

Comment 8

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Frank!
This issue has been tracked for Fx 98 and Fx99. However, Ting-Yu is away for two months. Can we have someone else on your team to take this instead? Thank you!

Comment 9

[Frank Griffith Jr]

I've spoken to Ting Yu about this. He's going to look into it through today and I'll ask Emilio if he can pick it up.

Comment 10

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I think this is an old bug because I can reproduce this on Nightly 2016-01-01. My patch in Bug 1550869 (landed in 2019-06-18) might hide it, and Bug 1745435 reveals it again.

Comment 11

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: testcase1.html

The testcase is simple: an image with pointer-events: none.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1754459 - Improve caret position selection for images. r=TYLin,jfkthame

What saves us in the non-pointer-events: none case is silly, and is
this loop that hits the draggable image:

https://searchfox.org/mozilla-central/rev/fa69d8b248e6c1df670aa6b019e30ec37e6672be/layout/generic/nsIFrame.cpp#4752

Needs tests (if it doesn't affect existing ones), but uploading for some
initial feedback. I think this makes a lot more sense than the existing
code, and works for editing and dragging the same way, so it's nice to
have less special-cases.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57845918277d
Improve caret position selection for images. r=dholbert

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/57845918277d

Comment 15

[Donal Meehan [:dmeehan]]

:emilio looks like a good candidate for a beta uplift request for 99?

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9268085 [details]
Bug 1754459 - Improve caret position selection for images. r=TYLin,jfkthame

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0. Also please re-test that bug 1550869 and bug 1745435 did not regress. I checked manually and we have tests for that but worth sanity-checking if we're going to uplift.
• List of other uplifts needed: Bug 1759866
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively simple change. This code is a bit gnarly though it's pretty self-contained so I'm mildly confident it won't cause regressions. Behavior also matches other browsers which is good.
• String changes made/needed: none

Comment 17

[Donal Meehan [:dmeehan]]

Comment on attachment 9268085 [details]
Bug 1754459 - Improve caret position selection for images. r=TYLin,jfkthame

Approved for 99.0b7. Thanks.

Comment 18

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/90ce3fa7b88e

Comment 19

[Simona Badau, Desktop QA]

Reproduced the initial issue on Firefox 99 beta 6 on Windows 10 x64.
Verified as fixed on the latest Nightly 100.0a1 on Windows 10 x 66 - the images are no longer highlighted on a single click.
Re-tested also that Bug 1550869 and Bug 1745435 did not regress on the latest Nightly 100.0a1.

Comment 20

[Marco]

Yes seems on Nightly 100.0a1 the issue is resolved.
Thanks.

Comment 21

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 99.0b7 on macOS Big Sur 11.6, Windows 10 x64, and Ubuntu 20.04 x64.

Comment 22

[Frank Griffith Jr]

Thanks for getting this across the line, emilio!