Bug 147316 – Web Inspector: Autocomplete: Undo (Cmd+Z) doesn't work as expected

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Created attachment 257547 [details]
Animated GIF of the problem

The problem persists in all the places where autocomplete is available: console, styles panel, CSS resource, etc.

Steps:
1. type "window" in the console
2. Press Command Z

Actual:
> window|w
Note that Command Z didn't undo anything. On the contrary, it added an extra "w" at the end.


Expected:
> |
Console is empty, Command Z removed the entire word. This is how it works in Xcode and any other code editor I have encountered.

[Radar WebKit Bug Importer]

<rdar://problem/22005227>

[Devin Rousso]

Created attachment 257895 [details]
[Proposed] Patch

The reason for this is because the "replaceRange" call in "WebInspector.CodeMirrorCompletionController._applyCompletionHint" causes CodeMirror to add another history state, and that addition messes up the previous history states (each time the user changes the text, replaceRange is called, therefore interrupting the collating of history events).  Using "setUniqueBookmark" allows us to keep the history state accurate while also displaying the current completion.  There are some small setbacks (it does not have all of the styling as before), but the benefits of being able to undo nicely outweigh those drawbacks (in my opinion at least).

[Nikita Vasilyev]

Created attachment 257984 [details]
[Animated GIF] With the patch applied

This works great!

(In reply to comment #2)
> Created attachment 257895 [details]
> [Proposed] Patch
> 
> ... There are some small setbacks (it does
> not have all of the styling as before)

Could you provide an example when this happens?

[Devin Rousso]

Created attachment 257986 [details]
After Patch is Applied: CSS Rules Sidebar

(In reply to comment #3)
> Could you provide an example when this happens?

This issue also occurred in the CSS rules sidebar and the text there has a purplish color.  Since the completion hint text is now inside a text marker and not part of the text in the editor, it does not get any of the styling (notice that the "mar" is purple and the "gin: " is grey).

[Nikita Vasilyev]

(In reply to comment #4)
> Created attachment 257986 [details]
> After Patch is Applied: CSS Rules Sidebar
> 
> (In reply to comment #3)
> > Could you provide an example when this happens?
> 
> This issue also occurred in the CSS rules sidebar and the text there has a
> purplish color.  Since the completion hint text is now inside a text marker
> and not part of the text in the editor, it does not get any of the styling
> (notice that the "mar" is purple and the "gin: " is grey).

The suggest hint used go be semi-transparent purple but now it's grey, I see.

(In reply to comment #2)
> ... There are some small setbacks (it does
> not have all of the styling as before), but the benefits of being able to
> undo nicely outweigh those drawbacks (in my opinion at least).

Agreed. I'd land this as it is and fix the suggest hint color later as a separate issue.

[Timothy Hatcher]

Comment on attachment 257895 [details]
[Proposed] Patch

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

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

We could fix the color by mimicking the color based on the current token. But I think it is fine as grey.

[WebKit Commit Bot]

Comment on attachment 257895 [details]
[Proposed] Patch

Clearing flags on attachment: 257895

Committed r187708: <http://trac.webkit.org/changeset/187708>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.