user-select: none shouldn't affect editability

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Consider merging https://chromium.googlesource.com/chromium/blink/+/986d2d7a2fea8566f91517ff4e3e42f0248ed6be

This patch makes CSS property |user-select:none| not to affect editability as IE and FireFox. 

However, selection behavior different among browsers:

- Chrome: selection works as |user-select:text|.
- FireFox: selection works as |user-select:text| for text input and textarea. In other elements, selection can be extended by cursor key. Mouse click always set caret to start of text.
- IE: Selection can not be extended neither cursor key nor mouse click.

* editing/selection/4866671.html: Replaced with editing/selection/user-select-none-in-editable.html

* editing/selection/5779984-1.html: Replaced with editing/selection/user-select-none-in-editable.html and user-select-js-property.html, for style.webkitUserSelect property.

* fast/events/standalone-image-drag-to-editable.html: Convert to text test instead of pixel test for updating reference images, since the final image is colored with selection color.

[Ferdy Christant]

This code....

*, *:before, *:after {
  user-select: none;
}

Is sometimes used by people to avoid text selection alltogether, mostly in a PWA or Add to Homescreen scenario. 

The above code completely blocks keyboard input on input fields. This is only an issue on mobile Safari, as far as I was able to test.

Is this 2013 bug I'm commenting on the same scope? Any plans to pick it up?

[Ryosuke Niwa]

(In reply to Ferdy Christant from comment #1)
> This code....
> 
> *, *:before, *:after {
>   user-select: none;
> }
> 
> Is sometimes used by people to avoid text selection alltogether, mostly in a
> PWA or Add to Homescreen scenario. 
> 
> The above code completely blocks keyboard input on input fields. This is
> only an issue on mobile Safari, as far as I was able to test.
> 
> Is this 2013 bug I'm commenting on the same scope? Any plans to pick it up?

This is indeed the bug that tracks this issue.

It's a bad idea to have such a style on the entire page since it would prevent users from selecting any text even if fixing this bug was a good idea.

[Ferdy Christant]

Agreed on both accounts: that this is a bad practice, and that regardless of the practice, it's still a bug.

[Tim Nguyen (:ntim)]

There's effectiveUserSelect() that can be changed to take in account user-modify.

[Tim Nguyen (:ntim)]

Created attachment 457879 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/91955044>

[Tim Nguyen (:ntim)]

*** Bug 82692 has been marked as a duplicate of this bug. ***

[Antti Koivisto]

Comment on attachment 457879 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457879&action=review

> Source/WebCore/rendering/style/RenderStyle.h:630
> +    UserSelect effectiveUserSelect() const { return effectiveInert() ? UserSelect::None : (userModify() == UserModify::ReadOnly ? userSelect() : UserSelect::Text); }

Maybe just move this out-of-line and make it multiline with ifs? Nested ternary operators are difficult to read.

> LayoutTests/ChangeLog:9
> +        - Added new tests for new behavior (imported from the Blink codebase)

Is a WPT possible?

[Tim Nguyen (:ntim)]

Created attachment 457900 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Tim Nguyen (:ntim)]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/33694

[Tim Nguyen (:ntim)]

Created attachment 457910 [details]
Patch

[EWS]

Committed r293028 (249767@main): <https://commits.webkit.org/249767@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457910 [details].