Bug 177981 – Web Inspector: Network Tab - Search Headers Detail View

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Network Tab - Search Headers Detail View

Should be able to search to find particular headers or search forward/back.

[Joseph Pecoraro]

Created attachment 322943 [details]
[IMAGE] Headers - search

[Joseph Pecoraro]

Created attachment 322944 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 323066 [details]
[PATCH] Proposed Fix

Rebaselined.

[Devin Rousso]

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]

Created attachment 323142 [details]
[PATCH] Proposed Fix

[Radar WebKit Bug Importer]

<rdar://problem/34877133>

[Blaze Burg]

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]

> 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]

<https://trac.webkit.org/r223057>