Bug 38649 - Web Inspector: implement panels history traversal on Cmd+Left/Right.
Summary: Web Inspector: implement panels history traversal on Cmd+Left/Right.
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: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-06 09:25 PDT by Pavel Feldman
Modified: 2010-05-06 10:18 PDT (History)
11 users (show)

See Also:


Attachments
[PATCH] Proposed change. (4.04 KB, patch)
2010-05-06 09:31 PDT, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-05-06 09:25:55 PDT
Patch to follow. I did not put a history object into a separate file, because it was too small. Will do it upon request. Although our poor parsers would need to parse more copyright comments!
Comment 1 Pavel Feldman 2010-05-06 09:31:24 PDT
Created attachment 55250 [details]
[PATCH] Proposed change.
Comment 2 Pavel Feldman 2010-05-06 09:53:50 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/inspector.js
Committed r58891
Comment 3 Joseph Pecoraro 2010-05-06 09:56:04 PDT
I just have a few NITs. Otherwise this patch looks awesome! Great idea.

>      switch (event.keyIdentifier) {
> +        case "Left":
> +            if (isMac)
> +                var isBackKey = event.metaKey;
> +            else
> +                var isBackKey = event.ctrlKey;

> +
> +        case "Right":
> +            if (isMac)
> +                var isForwardKey = event.metaKey;
> +            else
> +                var isForwardKey = event.ctrlKey;

I think a ternary operator in these cases would read much better here.

  var isBackKey = (isMac ? event.metaKey : event.ctrlKey);

This also seems like such a common task (isn't it used all over?) that it might
be worth putting into a method.



> +        this._history.splice(this._historyIterator + 1, this._history.length - this._historyIterator - 1);

The second parameter might not be required. I know that JavaScriptCore
implements the following "Spidermonkey Extension": (from MDC)

>  If no `howMany` parameter is specified (second syntax above, which is
>  a SpiderMonkey extension), all elements after index are removed.

Producing the simpler:

  this._history.splice(this._historyIterator + 1);
Comment 4 WebKit Review Bot 2010-05-06 10:14:50 PDT
http://trac.webkit.org/changeset/58891 might have broken GTK Linux 32-bit Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/58891
http://trac.webkit.org/changeset/58892
Comment 5 Timothy Hatcher 2010-05-06 10:18:29 PDT
Not this one.