Bug 181255 - Web Inspector: Find banner sometimes does not work (when already populated and shown for first time on resource)
Summary: Web Inspector: Find banner sometimes does not work (when already populated an...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-01-03 12:30 PST by Joseph Pecoraro
Modified: 2018-01-03 16:46 PST (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (2.71 KB, patch)
2018-01-03 12:41 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2018-01-03 12:30:40 PST
Find banner sometimes does not work (when already populated and shown for first time on resource)

Steps to Reproduce:
1. Inspect webkit.org
2. Show Debugger tab
3. Select "global.js"
4. Show Find Banner (⌘F)
5. Search for "document"
  => 3 results
6. Reload the page (⌘R)
7. Select global.js again if needed
8. Show Find Banner (⌘F)
  => Already populated with "document"
9. Hit Enter
  => No results

Notes:
- ContentBrowser owns the FindBanner and applies it to whatever ContentView it is showing
- When a ContentBrowser opens a TextContentView* for Resource A

  (1) TextContentView triggers Load Content of Resource A
    => NetworkAgent.getResponseBody / Debugger.getScriptSource
  (2) ContentBrowser triggers Perform Search with existing FindBanner value
    => Page.searchInResource / Debugger.searchInContent

While these execute on the backend serially, the result for getting the content body goes through multiple microtask loops. So (1) might be processed in the TextEditor after we attempt to process (2).
Comment 1 Joseph Pecoraro 2018-01-03 12:30:48 PST
<rdar://problem/36248855>
Comment 2 Joseph Pecoraro 2018-01-03 12:36:37 PST
ContentBrowser just does:

  1. Open ContentView
  2. ContentView.performSearch(query)

And here, text based ContentViews does asynchronous work in both of these:

  1.1 Load Content from backend
  2.1 Search Content on backend

---

So really, this can be a problem with automatically performing a search on any content view that loads its content asynchronously. In practice I think that is really only the content views containing a TextEditor, and those proxy the searching to TextEditor.

So for now, I'm just going to make TextEditor defer the search until it has loaded its initial content. Ensuring that when it does the search it should be able to show the results.

This also might have a minor perf win to avoid performing searches on resources every time you select them, even if the find banner is not visible.

---

We also appear to have an issue where ⌘E / ⌘G doesn't work unless the FindBanner is visible, which will be a separate bug.
Comment 3 Joseph Pecoraro 2018-01-03 12:41:06 PST
Created attachment 330411 [details]
[PATCH] Proposed Fix
Comment 4 Matt Baker 2018-01-03 16:41:33 PST
Comment on attachment 330411 [details]
[PATCH] Proposed Fix

r=me
Comment 5 WebKit Commit Bot 2018-01-03 16:46:56 PST
Comment on attachment 330411 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 330411

Committed r226380: <https://trac.webkit.org/changeset/226380>
Comment 6 WebKit Commit Bot 2018-01-03 16:46:57 PST
All reviewed patches have been landed.  Closing bug.