389283 - Text cursor value for cursor:auto depends on (ancestor) tabindex attribute

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Mats Palmgren (inactive)]

Text cursor value for cursor:auto depends on (ancestor) tabindex attribute.
This doesn't seem right to me.

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsTextFrameThebes.cpp&rev=3.54&root=/cvsroot&mark=3439-3454#3431
(it was added by bug 171366)

I think this block should either be removed, or replaced with code
that chooses a cursor based on if the text is selectable or not
(-moz-user-selectable).

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 2

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Choose cursor value based on IsSelectable()...

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 273458 [details] [diff] [review]
Patch rev. 1

This makes much more sense to me than the old code, although I think you should check the style like PeekOffsetWord and PeekOffsetCharacter do (-moz-user-select: all means, I think, that only the whole thing can be selected; see http://www.w3.org/TR/2000/WD-css3-userint-20000216#user-select ).  It's probably worth double-checking that this makes sense given how we actually behave with 'all'.

It looks like this was originally added for bug 171366.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

So I meet this issue 10 years latter... I don't understand why the patch didn't get landed 10 years ago. It makes lots of sense to me.

For handling of user-select: all, we currently display I-beam for element with that style, and WebKit and Blink do so as well, so I don't think we should change that behavior, even though user can't really select part of the text.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 389283 - Choose cursor value based on selectability.

Review commit: https://reviewboard.mozilla.org/r/95392/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/95392/

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

That patch is basically a rebase of that from comment 2. It seems the code doesn't change a lot over the past ten years :)

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8814087 [details]
Bug 389283 - Choose cursor value based on selectability.

https://reviewboard.mozilla.org/r/95392/#review96222

Tests might not be a bad idea... but r=dbaron.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

I have no idea how this can be tested...

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/756713ac1804
Choose cursor value based on selectability. r=dbaron

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/756713ac1804