Support search in iframe based webview #96307
Comments
mjbvz
added
feature-request
|
Solutions investigated:
|
|
I've explored various ways to implement this. There are two parts to this:
|
|
This iteration the plan on the runtime side is to explore extending the existing find service in chromium which handles frame based search pretty well, we only need to add a way to filter out per-frame results rather than results from the entire document. The level of implementation for this will vary depending on where we are carrying out this result filter (at content layer or implement at the WebFramMain layer in electron). It would be best if we landed on an implementation on the native side for performance reasons and ease of use. |
|
I've been changing around the Fiddle to understand the search behaviour better. Meanwhile, with iframes inside of iframes, all matches of the outer iframe are traversed first, before those of the inner iframe are traversed. Iframes inside of iframes are also loadable on Microsoft Edge, so here's how searching in nested iframes on Microsoft Edge (using their find widget, not setInterval) looks: I thought that I would have to consider these cases for notebook output cells, but it turns out there's already another issue, where notebook output cells currently can't be searched: #94239. Furthermore, I'm still unsure how focusing and getting the active match works in detail in the case of a single iframe with a search inputbox, and I plan on adding some logging statements later to get the values of some variables. |
|
Just for reference, here's another focus-related bug, but I'm unable to currently tell whether it's the same issue as this one: #113511 (comment) |
|
I added some logging. |
|
I looked more into two scenarios:
During the initial search, when a user types their first character into the textbox, both scenarios seem to proceed in the same execution order. However, when the user types a second character into the textbox, the behaviours differ. Scenario 1 In scenario 1, This calls into The Scenario 2 Because it is not called on the first pass, there is no active match found, and the Again, this calls into Therefore, if we want to support searching in an iframe, we:
A hypothesis I have is that focusing the frame is possible when searching in a webview, because a webview has its own |
|
It looks like we don't have to worry about point 2 above, due to Therefore, commenting out the frame-focusing lines, and setting The downsides are:
|
|
Thanks for the debugging info, agree with the maintainability part. This fix depends on an internal detail that could be changed between versions and having to spend time on this again is not useful. Conversely, based on the webview points in #96307 (comment) can you try this idea where we move the input box to a |
|
I created a PR to get concurrent iframe searches working again: electron/electron#29241 Also, here's a demo showing how if the input container is in a different webContents, searching works again: |
|
This morning, I learned from @bpasero that there's a I added a Screencap below:
Even if I were to add a debounce, this approach has the jumping search problem, where alternating the search string between "i" and "in" results in the active match jumping around, rather than staying in one place, due to the behaviour of Therefore, I think using a BrowserView would still be the best way forward for searching in iframes in the desktop app. |
|
Thanks for giving this a try. Can we ship this to users to get some initial feedback and investigate the browser view based solution separately? Maybe get some coverage in insiders to hear back from users how bad this is? |
|
I added a debounce, and here's how that looks:
🐛 I typed Ctrl+A in the middle of one of the searches, and got the search frame to focus. The jumping search problem still occurs (not shown in recording), though it's not as visible now, due to the delay between searching and the search results. |
|
I like it 👍 |
|
I still think this behaviour differs too much from the way the find widget currently works with webviews to be worth implementing. |
|
@mjbvz any chance we could get ea8c6b3 into our builds? I have iframe based webviews enabled via settings (
My rationale is that I would like to selfhost on @rzhao271 changes for enabling find in iframes, but currently that is impossible. |
Add search for iframe based webviews. Fixes #96307 Co-authored-by: Matt Bierner <[email protected]>
and others
For #83188
Problem
On desktop, VS Code's webviews using the electron
<webview>tag. This gives a nice api for searching inside a webviewHowever on web we use normal iframes for webviews. These currently don't support the search functionality because there is no equivalent API for searching inside a iframe.
This is one blocker for using normal iframes for the webviews in desktop VS Code: #83188
Request
Implement search for iframe based webviews. This should attempt to match the behavior of the existing search functionality as much as possible
/cc @deepak1556
The text was updated successfully, but these errors were encountered: