RESOLVED FIXED 192652
REGRESSION(r238599): Web Inspector: Clicking on text doesn't move text caret when editing innerHTML/tagName/attribute 
https://bugs.webkit.org/show_bug.cgi?id=192652
Summary REGRESSION(r238599): Web Inspector: Clicking on text doesn't move text caret ...
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
DetailsFormatted DiffDiff
[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
(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. I can't reproduce this either. Devin, did you successfully apply the patch?
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.