Bug 107944 – Web Inspector: implmenet Ctrl-Arrow/Ctrl-Backspace in DefaultTextEditor

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Andrey Lushnikov]

Support Ctrl-Arrow/Ctrl-Shift-Arrow/Ctrl-Backspace shortcuts in DefaultTextEditor. Current implementation makes them unpredictable and thus unusable.

[Andrey Lushnikov]

Created attachment 184740 [details]
Patch

[Pavel Feldman]

Comment on attachment 184740 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        and delete words. Their behaviour should be equal to Sublime 2's.

This should be fixed in content editable.

[Pavel Feldman]

Comment on attachment 184740 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:416
> +        this._shortcuts[WebInspector.KeyboardShortcut.makeKey(keys.Left.code, modifiers.CtrlOrMeta)] = this._handleCtrlArrow.bind(this, "left");

Do not affect meta.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:434
> +            return (char >= 'a' && char <= 'z') ||

what about unicode letters?

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:444
> +            return char === " ";

What about \t ?

[Andrey Lushnikov]

Created attachment 184752 [details]
Patch

[Andrey Lushnikov]

Created attachment 184754 [details]
Patch

[Pavel Feldman]

Comment on attachment 184754 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:417
> +        this._shortcuts[WebInspector.KeyboardShortcut.makeKey(keys.Right.code, modifiers.Ctrl)] = this._handleCtrlArrow.bind(this, "right");

It would be great if we could extract these into a common 'controller' subclass (located in the same file).

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:434
> +            var charCode = char.charCodeAt(0);

lets use string comparizon

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:454
> +        if (column === -1 && direction === "left") {

growRangeLeft/Right

[Andrey Lushnikov]

Created attachment 185229 [details]
Patch

[Andrey Lushnikov]

Comment on attachment 184754 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:417
>> +        this._shortcuts[WebInspector.KeyboardShortcut.makeKey(keys.Right.code, modifiers.Ctrl)] = this._handleCtrlArrow.bind(this, "right");
> 
> It would be great if we could extract these into a common 'controller' subclass (located in the same file).

fixed.

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:434
>> +            var charCode = char.charCodeAt(0);
> 
> lets use string comparizon

fixed.

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:454
>> +        if (column === -1 && direction === "left") {
> 
> growRangeLeft/Right

these methods are not applicable here, because they do different thing.

[WebKit Review Bot]

Comment on attachment 185229 [details]
Patch

Clearing flags on attachment: 185229

Committed r141245: <http://trac.webkit.org/changeset/141245>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Florin Malita]

(In reply to comment #9)
> (From update of attachment 185229 [details])
> Clearing flags on attachment: 185229
> 
> Committed r141245: <http://trac.webkit.org/changeset/141245>

This seems to be causing consistent timeouts for text-editor-ctrl-movements.html:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=inspector%2Feditor%2Ftext-editor-ctrl-movements.html

Can you please take a look?

[Florin Malita]

http://trac.webkit.org/changeset/141286

[Andrey Lushnikov]

Thanks for letting me know!

Created a bug to fix this: https://bugs.webkit.org/show_bug.cgi?id=108440