RESOLVED FIXED Bug 38649
Web Inspector: implement panels history traversal on Cmd+Left/Right.
https://bugs.webkit.org/show_bug.cgi?id=38649
Summary Web Inspector: implement panels history traversal on Cmd+Left/Right.
Pavel Feldman
Reported 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!
Attachments
[PATCH] Proposed change. (4.04 KB, patch)
2010-05-06 09:31 PDT, Pavel Feldman
timothy: review+
Pavel Feldman
Comment 1 2010-05-06 09:31:24 PDT
Created attachment 55250 [details] [PATCH] Proposed change.
Pavel Feldman
Comment 2 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
Joseph Pecoraro
Comment 3 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);
WebKit Review Bot
Comment 4 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
Timothy Hatcher
Comment 5 2010-05-06 10:18:29 PDT
Not this one.
Note You need to log in before you can comment on or make changes to this bug.