Bug 162564 – [GTK] Mac defaults are used for key shortcuts on Linux

• Component: Web Inspector
• Status: RESOLVED FIXED

[Tomas Popela]

Trying to use Control + Left/Right doesn't move the caret in console, but Alt + Left/Right does (where there is actually no web view's history item, otherwise it will move to it). The cause is that we are using the macDefault keymap as fallthrough for CodeMirror.

[Radar WebKit Bug Importer]

<rdar://problem/28475235>

[Tomas Popela]

Created attachment 289832 [details]
Proposed patch

[Carlos Garcia Campos]

Comment on attachment 289832 [details]
Proposed patch

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

> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:591
> -        fallthrough: "macDefault"
> +        fallthrough: mac ? "macDefault":"pcDefault"

leave space between :

[Tomas Popela]

Created attachment 289833 [details]
Fix style issue

[Matt Baker]

Comment on attachment 289833 [details]
Fix style issue

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

> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:575
> +    var mac = CodeMirror.keyMap["default"] == CodeMirror.keyMap.macDefault;

Style: strict equality (===) is used almost exclusively throughout the codebase. Also we've been preferring `let` over `var` for new code.

[Tomas Popela]

(In reply to comment #5)
> Comment on attachment 289833 [details]
> Fix style issue
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289833&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:575
> > +    var mac = CodeMirror.keyMap["default"] == CodeMirror.keyMap.macDefault;
> 
> Style: strict equality (===) is used almost exclusively throughout the
> codebase. Also we've been preferring `let` over `var` for new code.

Thanks, I will update the patch. I copy the code from CodeMirror, but I will use the detection that is used across the Inspector code:

let mac = WebInspector.Platform.name === "mac";

[Tomas Popela]

Created attachment 289841 [details]
Proposed patch

[Build Bot]

Comment on attachment 289841 [details]
Proposed patch

Attachment 289841 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2149548

New failing tests:
inspector/codemirror/prettyprinting-javascript.html
inspector/codemirror/prettyprinting-css-rules.html
inspector/codemirror/prettyprinting-css.html

[Build Bot]

Created attachment 289851 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 289841 [details]
Proposed patch

Attachment 289841 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2149551

New failing tests:
inspector/codemirror/prettyprinting-javascript.html
inspector/codemirror/prettyprinting-css-rules.html
inspector/codemirror/prettyprinting-css.html

[Build Bot]

Created attachment 289852 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 289841 [details]
Proposed patch

Attachment 289841 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2149549

New failing tests:
inspector/codemirror/prettyprinting-css.html

[Build Bot]

Created attachment 289857 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Blaze Burg]

Comment on attachment 289841 [details]
Proposed patch

Tests are failing because WebInspector.Platform is not defined in the test resource.

To fix this, edit Source/WebInspectorUI/UserInterface/Test.html, and include Platform.js right after InspectorFrontendHostStub.js

[Tomas Popela]

(In reply to comment #14)
> Comment on attachment 289841 [details]
> Proposed patch
> 
> Tests are failing because WebInspector.Platform is not defined in the test
> resource.
> 
> To fix this, edit Source/WebInspectorUI/UserInterface/Test.html, and include
> Platform.js right after InspectorFrontendHostStub.js

Thank you Brian, I will.

[Tomas Popela]

Created attachment 289921 [details]
Fix Mac tests

[WebKit Commit Bot]

Comment on attachment 289921 [details]
Fix Mac tests

Rejecting attachment 289921 [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-03', 'validate-changelog', '--check-oops', '--non-interactive', 289921, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebInspectorUI/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

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

[Tomas Popela]

Created attachment 289925 [details]
Fix the wrong 'reviewed by' phrase

[WebKit Commit Bot]

Comment on attachment 289925 [details]
Fix the wrong 'reviewed by' phrase

Clearing flags on attachment: 289925

Committed r206426: <http://trac.webkit.org/changeset/206426>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.