RESOLVED FIXED 146498
Web Inspector: Option+Click not jumping to resource 
https://bugs.webkit.org/show_bug.cgi?id=146498
Summary Web Inspector: Option+Click not jumping to resource
Joseph Pecoraro
Reported 2015-06-30 23:58:34 PDT
* SUMMARY Option+Click not jumping to resource. * STEPS TO REPRODUCE 1. Inspect <body> on <http://bogojoker.com> 2. In the Styles sidebar, Option+Click anything in the CSS (or the pattern.png resource) => no jump to resource
Attachments
Patch (11.33 KB, patch)
2015-07-06 20:01 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Patch (12.02 KB, patch)
2015-07-07 01:21 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2015-06-30 23:59:04 PDT
<rdar://problem/21627454>
Devin Rousso
Comment 2 2015-07-06 20:01:27 PDT
Created attachment 256271 [details] Patch
Timothy Hatcher
Comment 3 2015-07-06 21:07:21 PDT
Comment on attachment 256271 [details] Patch The force tab stuff was added because I was seeing tab switches during cookie restore. Maybe we don't need it now because cookie restore is only done when the tab is selected?
Joseph Pecoraro
Comment 4 2015-07-06 22:50:00 PDT
Comment on attachment 256271 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256271&action=review > Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorTokenTrackingController.js:355 > + if (this._delegate && typeof this._delegate.tokenTrackingControllerHighlightedRangeWasClicked === "function") > + this._delegate.tokenTrackingControllerHighlightedRangeWasClicked(this); This is not commented in the ChangeLog. We couldn't determine why the setTimeout was needed... but it looks like it was there to deal with CodeMirror. In any case, if you didn't see a regression with this change it seems find to make.
Joseph Pecoraro
Comment 5 2015-07-06 22:59:42 PDT
Comment on attachment 256271 [details] Patch Doing some quick searches I found a few call sites that were missed. (Maybe you just searched WebInspector.foo and missed these which don't 100% match that call site signature): Base/Main.js 1535: this.showMainFrameDOMTree(event.data.node, true); Base/Main.js 1138: this.showSourceCodeForFrame(frame.id, true); Thanks for looking into this!
Joseph Pecoraro
Comment 6 2015-07-06 23:05:03 PDT
(In reply to comment #3) > Comment on attachment 256271 [details] > Patch > > The force tab stuff was added because I was seeing tab switches during > cookie restore. Maybe we don't need it now because cookie restore is only > done when the tab is selected? That was exactly what I was thinking and why I suggested this approach to Devin. I cannot think of any case where we don't want to switch to a relevant tab when trying to show a new content view. The idea with window.event was clever, but I can't think of cases where we would trigger switching content views that wasn't somehow user gesture triggered. Even if we end up needing to keep/restore this parameter, I'd rather see it negated, and have the default behavior allow for switching tabs.
Devin Rousso
Comment 7 2015-07-07 01:21:10 PDT
Created attachment 256290 [details] Patch
WebKit Commit Bot
Comment 8 2015-07-07 10:26:52 PDT
Comment on attachment 256290 [details] Patch Clearing flags on attachment: 256290 Committed r186466: <http://trac.webkit.org/changeset/186466>
WebKit Commit Bot
Comment 9 2015-07-07 10:26:57 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.