Bug 193218 - Web Inspector: Styles: clicking on property that soon to be discarded shouldn't start selection
Summary: Web Inspector: Styles: clicking on property that soon to be discarded shouldn...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-07 15:12 PST by Nikita Vasilyev
Modified: 2019-01-09 00:34 PST (History)
5 users (show)

See Also:

Attachments
[Video] Bug (389.94 KB, video/quicktime)
2019-01-07 15:12 PST, Nikita Vasilyev 		no flags Details
Patch (2.92 KB, patch)
2019-01-07 15:27 PST, Nikita Vasilyev 		no flags Details | Formatted Diff | Diff
[Video] With patch applied (1.77 MB, video/quicktime)
2019-01-07 15:34 PST, Nikita Vasilyev 		no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (54.75 MB, application/zip)
2019-01-07 18:18 PST, EWS Watchlist 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2019-01-07 15:12:02 PST 
Created attachment 358538 [details]
[Video] Bug

Steps:
1. Add a new property "float"
2. Press Tab
3. Click on the white space on the right side of "float"

Expected:
The newly added property get discarded (because it has an empty value).

Actual:
The newly added property get discarded (because it has an empty value), but moving mouse cursor now selects properties.
Comment 1 Radar WebKit Bug Importer 2019-01-07 15:18:46 PST 
<rdar://problem/47098303>
Comment 2 Nikita Vasilyev 2019-01-07 15:27:49 PST 
Created attachment 358542 [details]
Patch
Comment 3 Nikita Vasilyev 2019-01-07 15:34:07 PST 
Created attachment 358545 [details]
[Video] With patch applied

This fixes the bug and doesn't break anything else (that I know of :)).
Comment 4 EWS Watchlist 2019-01-07 18:17:50 PST 
Comment on attachment 358542 [details]
Patch

Attachment 358542 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10665029

New failing tests:
imported/w3c/web-platform-tests/webrtc/simplecall-no-ssrcs.https.html
Comment 5 EWS Watchlist 2019-01-07 18:18:12 PST 
Created attachment 358562 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 6 Nikita Vasilyev 2019-01-07 18:28:10 PST 
Comment on attachment 358542 [details]
Patch

Unrelated test failure.
Comment 7 Devin Rousso 2019-01-08 18:54:22 PST 
Comment on attachment 358542 [details]
Patch

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

r=me, nice refactor/fix

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:414
> +            this._stopSelection();

Rather than set `_isMousePressed` to `true` and then almost immediately reset it back to `false`, could we early return:

    if (!propertyElement || !propertyElement.parentNode)
Comment 8 Nikita Vasilyev 2019-01-09 00:08:52 PST 
Comment on attachment 358542 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:414
>> +            this._stopSelection();
> 
> Rather than set `_isMousePressed` to `true` and then almost immediately reset it back to `false`, could we early return:
> 
>     if (!propertyElement || !propertyElement.parentNode)

`_isMousePressed` needs to be set to `true` before "blur" event triggered by `document.activeElement.blur()` or change of selection.
Comment 9 WebKit Commit Bot 2019-01-09 00:34:23 PST 
Comment on attachment 358542 [details]
Patch

Clearing flags on attachment: 358542

Committed r239766: <https://trac.webkit.org/changeset/239766>
Comment 10 WebKit Commit Bot 2019-01-09 00:34:24 PST 
All reviewed patches have been landed.  Closing bug.