Bug 164391

Summary:

REGRESSION (r208248): Web Inspector: Pressing Left Arrow breaks autocomplete

Product:

WebKit

Reporter:

Nikita Vasilyev <nvasilyev>

Component:

Web Inspector

Assignee:

Nikita Vasilyev <nvasilyev>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, commit-queue, hi, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
[Animated GIF] Bug	none
Patch	none
[Animated GIF] With patch applied	none
Patch	none

Description Nikita Vasilyev 2016-11-03 17:48:59 PDT

Steps:
1. Open Console.
2. Type "a"
3. Press Arrow Down key.
4. Press Arrow Left key.
5. Erase console prompt (by pressing Cmd+A and Delete, for example).
6. Type "a".

Expected:
a|ddEventListener (Autocomple still works)


Actual:
a| (Autocomplete doesn't work any more)

Notes:
This is a recent regression. It works as expected in Safari Technology Preview 16.

Comment 1 Radar WebKit Bug Importer 2016-11-03 17:49:24 PDT

<rdar://problem/29102408>

Comment 2 Nikita Vasilyev 2016-11-03 17:55:32 PDT

Created attachment 293838 [details]
[Animated GIF] Bug

Comment 3 Nikita Vasilyev 2016-11-15 12:53:57 PST

This regressed in https://trac.webkit.org/changeset/208248,
which in turn was supposed fix another autocomplete regression.

Comment 4 Nikita Vasilyev 2016-11-15 15:36:25 PST

Created attachment 294887 [details]
Patch

Unroll r208248.

Comment 5 Nikita Vasilyev 2016-11-15 15:36:53 PST

Created attachment 294888 [details]
[Animated GIF] With patch applied

Comment 6 Nikita Vasilyev 2016-11-15 15:40:23 PST

Created attachment 294890 [details]
Patch

Comment 7 Matt Baker 2016-11-15 15:56:50 PST

Comment on attachment 294890 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:164
> +        delete this._ignoreNextCursorActivity;

Why were the two remaining properties changed to delete?

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:250
> +        var container = document.createElement("span");

Nit: let

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:251
> +        container.classList.add(WebInspector.CodeMirrorCompletionController.CompletionHintStyleClassName);

Since the class name was removed from CodeMirrorCompletionController.prototype._applyCompletionHint I think we can just inline the class name.

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:273
> +            var currentText = this._codeMirror.getRange(from, cursor);

Why did these change from let to var?

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:283
> +        delete this._ignoreChange;

Why the change?

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:322
> +        var history = this._codeMirror.getHistory();

Nit: let

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:381
> +        delete this._ignoreChange;

Why the change?

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:835
> +            delete this._ignoreNextCursorActivity;

Same as above.

Comment 8 Matt Baker 2016-11-15 15:57:32 PST

Doh! Nevermind.

Comment 9 WebKit Commit Bot 2016-11-15 17:03:30 PST

Comment on attachment 294890 [details]
Patch

Clearing flags on attachment: 294890

Committed r208774: <http://trac.webkit.org/changeset/208774>

Comment 10 WebKit Commit Bot 2016-11-15 17:03:40 PST

All reviewed patches have been landed.  Closing bug.