Bug 192652

Summary:

REGRESSION(r238599): Web Inspector: Clicking on text doesn't move text caret when editing innerHTML/tagName/attribute

Product:

WebKit

Reporter:

Nikita Vasilyev <nvasilyev>

Component:

Web Inspector

Assignee:

Matt Baker <mattbaker>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

chris, commit-queue, hi, inspector-bugzilla-changes, mattbaker, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

See Also:

https://bugs.webkit.org/show_bug.cgi?id=208310

Attachments:

Description	Flags
[Video] Bug	none
Patch	none
[Video] Multiple elements selected	none
[Video] Unable to change selection with mouse	none
[Video] Change selection with mouse	none

Nikita Vasilyev

Reported 2018-12-12 19:09:50 PST

Created attachment 357202 [details] [Video] Bug Steps: 1. Open https://webkit.org 2. Inspect <body> 3. Right click on <body> 4. Select "Edit - Attribute", "Edit - HTML", or "Edit - Tag" 5. Click in the middle of the editable field Expected: Text caret moves where the click was. Actual: Text caret doesn't move. Notes: The same thing happens when double-clicking on HTML attribute.

Attachments
[Video] Bug (1013.63 KB, video/quicktime) 2018-12-12 19:09 PST, Nikita Vasilyev	no flags	Details
Patch (3.45 KB, patch) 2019-01-16 12:51 PST, Matt Baker	no flags	Details Formatted Diff Diff
[Video] Multiple elements selected (3.09 MB, video/quicktime) 2019-01-16 13:35 PST, Nikita Vasilyev	no flags	Details
[Video] Unable to change selection with mouse (1.87 MB, video/quicktime) 2019-01-22 16:30 PST, Devin Rousso	no flags	Details
[Video] Change selection with mouse (1.55 MB, video/quicktime) 2019-01-25 14:31 PST, Matt Baker	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2018-12-12 19:10:13 PST

<rdar://problem/46684612>

Matt Baker

Comment 2 2019-01-16 12:51:01 PST

Created attachment 359294 [details] Patch

Matt Baker

Comment 3 2019-01-16 12:54:31 PST

Regressed in https://trac.webkit.org/changeset/238599.

Nikita Vasilyev

Comment 4 2019-01-16 13:14:00 PST

Looks good, I'd r+. In r238599, event.preventDefault() was called on mousedown even when DOMTreeElement was being edited. This patch fixes it.

Nikita Vasilyev

Comment 5 2019-01-16 13:35:58 PST

Created attachment 359298 [details] [Video] Multiple elements selected This patch caused multiple elements selected by mistake to come back.

Nikita Vasilyev

Comment 6 2019-01-16 13:41:24 PST

(In reply to Nikita Vasilyev from comment #5) > Created attachment 359298 [details] > [Video] Multiple elements selected > > This patch caused multiple elements selected by mistake to come back. I don't have reliable steps to reproduce it. I got into this state on the video after reloading https://webkit.org. I haven't seen this bug for a few weeks so I assumed it's a regression in your patch.

Matt Baker

Comment 7 2019-01-16 14:24:39 PST

(In reply to Nikita Vasilyev from comment #6) > (In reply to Nikita Vasilyev from comment #5) > > Created attachment 359298 [details] > > [Video] Multiple elements selected > > > > This patch caused multiple elements selected by mistake to come back. > > I don't have reliable steps to reproduce it. I got into this state on the > video after reloading https://webkit.org. I haven't seen this bug for a few > weeks so I assumed it's a regression in your patch. I'm not able to reproduce this. I tried clicking around various DOM nodes, using up/down arrows, expanding, etc. Could you adds steps?

Nikita Vasilyev

Comment 8 2019-01-16 14:52:38 PST

Comment on attachment 359294 [details] Patch I don't have the exact steps. I'll file a bug if I come up with a reliable way to reproduce it. I'm not sure it was related to the patch, so I'm reseting it back to "r?".

Devin Rousso

Comment 9 2019-01-22 16:30:07 PST

Created attachment 359804 [details] [Video] Unable to change selection with mouse I'm trying to click/drag in the video, and the selection doesn't respond. Using the keyboard works as expected.

Devin Rousso

Comment 10 2019-01-22 16:31:47 PST

Comment on attachment 359294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359294&action=review r-, see attachment 359804 [details] (comment #9) > Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:649 > // Prevent selecting the nearest word on double click. Should we only be doing this when we aren't editing? I can imagine that it would be very useful to double-click to select a specific class name (in the space-separated list) when editing.

Matt Baker

Comment 11 2019-01-22 18:27:28 PST

Comment on attachment 359294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359294&action=review >> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:649 >> // Prevent selecting the nearest word on double click. > > Should we only be doing this when we aren't editing? I can imagine that it would be very useful to double-click to select a specific class name (in the space-separated list) when editing. Originally (before multi-selection was introduced in TreeOutline) this was _only_ done when not editing: selectOnMouseDown(event) { super.selectOnMouseDown(event); if (this._editing) return; // Prevent selecting the nearest word on double click. if (event.detail >= 2) event.preventDefault(); }

Matt Baker

Comment 12 2019-01-25 14:31:57 PST

Created attachment 360165 [details] [Video] Change selection with mouse I can't reproduce the selection bug you demonstrated. Tried with the patch applied on top of 86ca8ae2d1c.

Devin Rousso

Comment 13 2019-01-25 16:44:35 PST

(In reply to Matt Baker from comment #12) > Created attachment 360165 [details] > [Video] Change selection with mouse > > I can't reproduce the selection bug you demonstrated. Tried with the patch > applied on top of 86ca8ae2d1c. Do you have any settings (e.g. "Press Tab to highlight each item on a webpage", or some accessibility setting in System Preferences)? I remember we had a issue with that in the past (tabbing through items in the Styles sidebar), so maybe this is also related to that.

Matt Baker

Comment 14 2019-01-27 19:35:02 PST

(In reply to Devin Rousso from comment #13) > (In reply to Matt Baker from comment #12) > > Created attachment 360165 [details] > > [Video] Change selection with mouse > > > > I can't reproduce the selection bug you demonstrated. Tried with the patch > > applied on top of 86ca8ae2d1c. > Do you have any settings (e.g. "Press Tab to highlight each item on a > webpage", or some accessibility setting in System Preferences)? I remember > we had a issue with that in the past (tabbing through items in the Styles > sidebar), so maybe this is also related to that. I have "Tap to click" enabled in my trackpad settings but that doesn't matter.

Nikita Vasilyev

Comment 15 2019-01-30 14:05:19 PST

Devin Rousso

Comment 16 2019-01-31 14:03:12 PST

Comment on attachment 359294 [details] Patch r=me Interesting. I reapplied this, and it seems to work fine now 🤔

WebKit Commit Bot

Comment 17 2019-01-31 14:17:54 PST

Comment on attachment 359294 [details] Patch Clearing flags on attachment: 359294 Committed r240819: <https://trac.webkit.org/changeset/240819>

WebKit Commit Bot

Comment 18 2019-01-31 14:17:56 PST

All reviewed patches have been landed. Closing bug.

Nikita Vasilyev

Comment 19 2019-02-02 14:00:17 PST

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

Note You need to log in before you can comment on or make changes to this bug.