Bug 101026 - Web Inspector: Settings screen: some panel shortcuts are missing
Summary: Web Inspector: Settings screen: some panel shortcuts are missing
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: eustas.bug
URL:
Keywords:
Depends on: 101301 102476
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-02 02:06 PDT by eustas.bug
Modified: 2012-11-21 23:44 PST (History)
11 users (show)

See Also:


Attachments
Missing shortcuts screesnhot (69.25 KB, image/png)
2012-11-02 02:06 PDT, eustas.bug
no flags Details
Patch (5.34 KB, patch)
2012-11-02 02:20 PDT, eustas.bug
no flags Details | Formatted Diff | Diff
Patch (50.00 KB, patch)
2012-11-06 03:57 PST, eustas.bug
no flags Details | Formatted Diff | Diff
Patch (50.00 KB, patch)
2012-11-06 04:01 PST, eustas.bug
no flags Details | Formatted Diff | Diff
Patch (51.63 KB, patch)
2012-11-08 02:30 PST, eustas.bug
no flags Details | Formatted Diff | Diff
Patch (51.65 KB, patch)
2012-11-13 03:25 PST, eustas.bug
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description eustas.bug 2012-11-02 02:06:26 PDT
Created attachment 172006 [details]
Missing shortcuts screesnhot

Panels, that hasn't been shown at the moment when shortcuts
view is initialized, had no chance to register shortcuts.
Comment 1 eustas.bug 2012-11-02 02:20:00 PDT
Created attachment 172010 [details]
Patch
Comment 2 Pavel Feldman 2012-11-02 05:31:07 PDT
Comment on attachment 172010 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Panels, that hasn't been shown at the moment when shortcuts

Consider rephrasing: .. that have not been loaded by the time shortcuts view was ..

> Source/WebCore/inspector/front-end/ShortcutsScreen.js:35
> +WebInspector.ShortcutsScreen = function(registrationCallback)

registrationCallback sounds weird too generic.

> Source/WebCore/inspector/front-end/inspector.js:653
> +        panelDescriptors[i].panel();

We should not create panels just to register their shortcuts. Instead, shortcut declaration code should be moved into the panel descriptors.
Comment 3 eustas.bug 2012-11-06 03:57:17 PST
Created attachment 172547 [details]
Patch
Comment 4 eustas.bug 2012-11-06 04:01:13 PST
Created attachment 172548 [details]
Patch
Comment 5 eustas.bug 2012-11-06 04:01:30 PST
Comment on attachment 172010 [details]
Patch

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

>> Source/WebCore/ChangeLog:8
>> +        Panels, that hasn't been shown at the moment when shortcuts
> 
> Consider rephrasing: .. that have not been loaded by the time shortcuts view was ..

Done.

>> Source/WebCore/inspector/front-end/ShortcutsScreen.js:35
>> +WebInspector.ShortcutsScreen = function(registrationCallback)
> 
> registrationCallback sounds weird too generic.

Removed.

>> Source/WebCore/inspector/front-end/inspector.js:653
>> +        panelDescriptors[i].panel();
> 
> We should not create panels just to register their shortcuts. Instead, shortcut declaration code should be moved into the panel descriptors.

Fixed.
Comment 6 Yury Semikhatsky 2012-11-07 23:51:43 PST
Comment on attachment 172548 [details]
Patch

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

> Source/WebCore/ChangeLog:14
> +        More chanhes: add JsDoc annotations to ShortcutScreen and change "key"

typo: chanhes-> changes

> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:76
> +        var incrementBy10 = WebInspector.ElementsPanelDescriptor.ShortcutKeys.IncrementValue.concat(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementValue);

Remove this line.

> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:79
> +        stylesPaneSection.addAlternateKeys(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementBy10, WebInspector.UIString("Decrement by %f", 10));

We should be consistent and use single entry "Increment/Decrement by 10" here as for 0.1 and 100

> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:81
> +        var incrementDecrementBy100 = WebInspector.ElementsPanelDescriptor.ShortcutKeys.IncrementBy01.concat(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementBy01);

incrementDecrementBy100 -> incrementDecrementBy01

> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:84
> +        var incrementDecrementBy01 = WebInspector.ElementsPanelDescriptor.ShortcutKeys.IncrementBy100.concat(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementBy100);

incrementDecrementBy01 -> incrementDecrementBy100. Looks like there is a mistake in shortcut description for .1 and 100
Comment 7 eustas.bug 2012-11-08 02:26:48 PST
Comment on attachment 172548 [details]
Patch

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

>> Source/WebCore/ChangeLog:14
>> +        More chanhes: add JsDoc annotations to ShortcutScreen and change "key"
> 
> typo: chanhes-> changes

Done.

>> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:76
>> +        var incrementBy10 = WebInspector.ElementsPanelDescriptor.ShortcutKeys.IncrementValue.concat(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementValue);
> 
> Remove this line.

Done.

>> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:79
>> +        stylesPaneSection.addAlternateKeys(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementBy10, WebInspector.UIString("Decrement by %f", 10));
> 
> We should be consistent and use single entry "Increment/Decrement by 10" here as for 0.1 and 100

Fixed.

>> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:81
>> +        var incrementDecrementBy100 = WebInspector.ElementsPanelDescriptor.ShortcutKeys.IncrementBy01.concat(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementBy01);
> 
> incrementDecrementBy100 -> incrementDecrementBy01

Fixed.

>> Source/WebCore/inspector/front-end/ElementsPanelDescriptor.js:84
>> +        var incrementDecrementBy01 = WebInspector.ElementsPanelDescriptor.ShortcutKeys.IncrementBy100.concat(WebInspector.ElementsPanelDescriptor.ShortcutKeys.DecrementBy100);
> 
> incrementDecrementBy01 -> incrementDecrementBy100. Looks like there is a mistake in shortcut description for .1 and 100

Fixed.
Comment 8 eustas.bug 2012-11-08 02:30:08 PST
Created attachment 172969 [details]
Patch
Comment 9 WebKit Review Bot 2012-11-13 03:06:19 PST
Comment on attachment 172969 [details]
Patch

Rejecting attachment 172969 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
rging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 [Qt] Unreviewed Qt gardening.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 154.

Full output: http://queues.webkit.org/results/14824260
Comment 10 eustas.bug 2012-11-13 03:25:59 PST
Created attachment 173857 [details]
Patch

rebased
Comment 11 WebKit Review Bot 2012-11-13 05:51:32 PST
Comment on attachment 173857 [details]
Patch

Clearing flags on attachment: 173857

Committed r134404: <http://trac.webkit.org/changeset/134404>
Comment 12 eustas.bug 2012-11-15 07:18:38 PST
Comment on attachment 172969 [details]
Patch

Cleanup after commit
Comment 13 Vsevolod Vlasov 2012-11-16 03:52:06 PST
Reopened since it was rolled out