Bug 80474 - Web Inspector: Cannot insert right curly bracket on some keyboards
Summary: Web Inspector: Cannot insert right curly bracket on some keyboards
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-06 19:34 PST by Cem Kocagil
Modified: 2012-03-08 12:34 PST (History)
11 users (show)

See Also:


Attachments
Patch (1.37 KB, patch)
2012-03-06 19:42 PST, Cem Kocagil
pfeldman: review-
Details | Formatted Diff | Diff
Patch (2.33 KB, patch)
2012-03-07 08:40 PST, Cem Kocagil
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cem Kocagil 2012-03-06 19:34:21 PST
Right curly brackets cannot be typed on keyboards where the right curly bracket is inserted by using AltGr+0. Instead, this combination has the same effect as ctrl+0 (reset zoom).

The changeset that causes the bug: http://trac.webkit.org/changeset/108794/

Patch is on the way
Comment 1 Cem Kocagil 2012-03-06 19:42:19 PST
Created attachment 130523 [details]
Patch
Comment 2 Alexander Pavlov (apavlov) 2012-03-07 01:11:08 PST
Comment on attachment 130523 [details]
Patch

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

> Source/WebCore/inspector/front-end/inspector.js:720
> +    var hasCtrlOrMeta = WebInspector.KeyboardShortcut.eventHasCtrlOrMeta(event) && !event.shiftKey && !event.altKey;

I would rename the variable into something like "hasOnlyCtrlOrMeta" in order to differentiate it from the "normal" hasCtrlOrMeta checks around the code. Otherwise looks good; thanks for diagnosing and fixing this.
Comment 3 Pavel Feldman 2012-03-07 01:47:23 PST
Comment on attachment 130523 [details]
Patch

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

>> Source/WebCore/inspector/front-end/inspector.js:720
>> +    var hasCtrlOrMeta = WebInspector.KeyboardShortcut.eventHasCtrlOrMeta(event) && !event.shiftKey && !event.altKey;
> 
> I would rename the variable into something like "hasOnlyCtrlOrMeta" in order to differentiate it from the "normal" hasCtrlOrMeta checks around the code. Otherwise looks good; thanks for diagnosing and fixing this.

Thanks for looking into it. As Alexander suggests, I'd move hasCtrlOrMeta closer to its usage (below the first switch), combined it with the !InspectorFrontendHost.isStub and renamed to something like isValidZoomShortcut.
Comment 4 Cem Kocagil 2012-03-07 08:40:39 PST
Created attachment 130631 [details]
Patch
Comment 5 Cem Kocagil 2012-03-08 05:09:17 PST
Updated, waiting for review
Comment 6 WebKit Review Bot 2012-03-08 12:34:41 PST
Comment on attachment 130631 [details]
Patch

Clearing flags on attachment: 130631

Committed r110192: <http://trac.webkit.org/changeset/110192>
Comment 7 WebKit Review Bot 2012-03-08 12:34:46 PST
All reviewed patches have been landed.  Closing bug.