WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
177981
Web Inspector: Network Tab - Search Headers Detail View
https://bugs.webkit.org/show_bug.cgi?id=177981
Summary
Web Inspector: Network Tab - Search Headers Detail View
Joseph Pecoraro
Reported
2017-10-05 17:19:29 PDT
Network Tab - Search Headers Detail View Should be able to search to find particular headers or search forward/back.
Attachments
[IMAGE] Headers - search
(223.42 KB, image/png)
2017-10-05 17:19 PDT
,
Joseph Pecoraro
no flags
Details
[PATCH] Proposed Fix
(9.58 KB, patch)
2017-10-05 17:21 PDT
,
Joseph Pecoraro
no flags
Details
Formatted Diff
Diff
[PATCH] Proposed Fix
(9.60 KB, patch)
2017-10-06 16:52 PDT
,
Joseph Pecoraro
no flags
Details
Formatted Diff
Diff
[PATCH] Proposed Fix
(9.37 KB, patch)
2017-10-08 14:38 PDT
,
Joseph Pecoraro
bburg
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1
2017-10-05 17:19:48 PDT
Created
attachment 322943
[details]
[IMAGE] Headers - search
Joseph Pecoraro
Comment 2
2017-10-05 17:21:16 PDT
Created
attachment 322944
[details]
[PATCH] Proposed Fix
Joseph Pecoraro
Comment 3
2017-10-06 16:52:20 PDT
Created
attachment 323066
[details]
[PATCH] Proposed Fix Rebaselined.
Devin Rousso
Comment 4
2017-10-06 21:37:21 PDT
Comment on
attachment 323066
[details]
[PATCH] Proposed Fix View in context:
https://bugs.webkit.org/attachment.cgi?id=323066&action=review
I'm pretty unfamiliar with the way we do search/find banners, so I am probably not the best to review this. :(
> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:419 > + let match = searchRegex.exec(text); > + while (match) { > + matchRanges.push({offset: match.index, length: match[0].length}); > + match = searchRegex.exec(text); > + }
You could borrow a thing from C++ and assign inside the condition. let match = null; while (match = searchRegex.exec(text)) matchRanges.push({offset: match.index, length: match[0].length});
> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:441 > + this._bouncyHighlightElement = document.createElement("div"); > + this._bouncyHighlightElement.className = "bouncy-highlight"; > + this._bouncyHighlightElement.textContent = highlightElement.textContent;
This seems to be the pattern we follow with `_boundyHighlightElement`, but it seems like a waste to keep creating an element with the exact same style/function. Instead of recreating the element, we could just remove it, set the text/top/left, and then add to have it restart the animation.
Joseph Pecoraro
Comment 5
2017-10-08 14:38:34 PDT
Created
attachment 323142
[details]
[PATCH] Proposed Fix
Radar WebKit Bug Importer
Comment 6
2017-10-08 15:19:06 PDT
<
rdar://problem/34877133
>
Blaze Burg
Comment 7
2017-10-09 10:26:16 PDT
Comment on
attachment 323142
[details]
[PATCH] Proposed Fix View in context:
https://bugs.webkit.org/attachment.cgi?id=323142&action=review
r=me We might benefit from some sort of SearchController[Delegate] to extract out the common search related state thats duplicated in each content view that overrides the various ContentView search methods. That might give us a uniform search delegate protocol between ContentViews and Sidebars.
> Source/WebInspectorUI/ChangeLog:11 > + Rename "Dom" to "DOM" in utility function.
OK
> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:43 > + this._searchDOMChanges = [];
Nit: searchDOMHighlightChanges to match other content views?
> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:428 > + let highlightElement = this._searchResults[index];
Nit: highlightedElement
Joseph Pecoraro
Comment 8
2017-10-09 11:10:36 PDT
> We might benefit from some sort of SearchController[Delegate] to extract out > the common search related state thats duplicated in each content view that > overrides the various ContentView search methods. That might give us a > uniform search delegate protocol between ContentViews and Sidebars.
I agree. This was a little tricky, if this pops up in yet another place (its only 2 right now in the Console and here) I'll look into doing this. That said, each place has a bit of custom logic that may make this a bit messy.
Joseph Pecoraro
Comment 9
2017-10-09 12:28:34 PDT
<
https://trac.webkit.org/r223057
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug