Bug 154719 – Web Inspector: Increase clickable area of the console prompt

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

[Nikita Vasilyev]

Created attachment 272300 [details]
[Image] Before/after

Currently, only the middle part of the console prompt is clickable.
When I click on ">" icon or the white space just below or above
the CodeMirror element, the console prompt loses focus. Instead,
it should keep the focus (or focus on the prompt, if it wasn't
focused before).

[Radar WebKit Bug Importer]

<rdar://problem/24854538>

[Nikita Vasilyev]

Created attachment 272301 [details]
Patch

[Timothy Hatcher]

Comment on attachment 272301 [details]
Patch

Not a patch. Is an image.

[Nikita Vasilyev]

Created attachment 272370 [details]
Patch

Whoops. Now it's a patch.

[Timothy Hatcher]

Comment on attachment 272370 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/QuickConsole.css:57
> +    pointer-events: none;

Why is this needed?

> Source/WebInspectorUI/UserInterface/Views/QuickConsole.js:113
> +        requestAnimationFrame(() => this.prompt.focus());

Why does this need a requestAnimationFrame?

[Nikita Vasilyev]

Comment on attachment 272370 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/QuickConsole.css:57
>> +    pointer-events: none;
> 
> Why is this needed?

In _handleMouseDown I do this to avoid capturing all the mousedown events from CodeMirror:

    if (event.target !== this.element)

If I don't use

    .quick-console > .console-prompt {pointer-events: none}

event.target could either be .quick-console or .console-prompt.
Setting `pointer-events: none` makes event.target to be always .quick-console.

>> Source/WebInspectorUI/UserInterface/Views/QuickConsole.js:113
>> +        requestAnimationFrame(() => this.prompt.focus());
> 
> Why does this need a requestAnimationFrame?

Without it, the prompt loses focus right after it gets it.
However, event.preventDefault() seems like a better solution.

[Timothy Hatcher]

Comment on attachment 272370 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/QuickConsole.js:113
>>> +        requestAnimationFrame(() => this.prompt.focus());
>> 
>> Why does this need a requestAnimationFrame?
> 
> Without it, the prompt loses focus right after it gets it.
> However, event.preventDefault() seems like a better solution.

Yeah, lets try with event.preventDefault() instead.

[Joseph Pecoraro]

Comment on attachment 272370 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/QuickConsole.css:57
>>> +    pointer-events: none;
>> 
>> Why is this needed?
> 
> In _handleMouseDown I do this to avoid capturing all the mousedown events from CodeMirror:
> 
>     if (event.target !== this.element)
> 
> If I don't use
> 
>     .quick-console > .console-prompt {pointer-events: none}
> 
> event.target could either be .quick-console or .console-prompt.
> Setting `pointer-events: none` makes event.target to be always .quick-console.

When adding something like this there should really be a comment in the ChangeLog, as it is non-obvious.

> Source/WebInspectorUI/UserInterface/Views/QuickConsole.js:42
> +        this.element.addEventListener("mousedown", this._handleMouseDown.bind(this), false);

Style: We have been leaving off the "false" on addEventListener calls that use the normal bubbling phase.

[Nikita Vasilyev]

Created attachment 272402 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 272402 [details]
Patch

Rejecting attachment 272402 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 272402, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ng file Source/WebInspectorUI/ChangeLog
Hunk #1 FAILED at 4.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebInspectorUI/ChangeLog.rej
patching file Source/WebInspectorUI/UserInterface/Views/QuickConsole.js
Hunk #1 FAILED at 39.
Hunk #2 FAILED at 110.
2 out of 2 hunks FAILED -- saving rejects to file Source/WebInspectorUI/UserInterface/Views/QuickConsole.js.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/889130

[Nikita Vasilyev]

Created attachment 272410 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 272410 [details]
Patch

Clearing flags on attachment: 272410

Committed r197245: <http://trac.webkit.org/changeset/197245>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.