1570965 - Remove the subframes argument in nsWebBrowserFind as it doesn't work and is also not supported by other browsers

Status: NEW

(Core :: Find Backend, enhancement, P3)

Description

[Neil Deakin]

This is used for window.find(). It currently iterates over docshells so doesn't iterate over subframes. There are several options.

1. Convert it to call into the same code that the findbar uses, which is being done by bug 1553384. This has the advantage of removing lots of code.
2. No longer support the subframes argument to window.find(). Neither Chrome nor Safari appear to handle it. This approach has the advantage that it requires no changes, although code to handle this could be removed.

nsWebBrowserFind is also used for a special case for View Selection Source, but that doesn't need to iterate over subframes, and I suspect it could be converted to just using nsIFind directly.

Comment 1

[Chris Peterson [:cpeterson]]

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Comment 2

[Chris Peterson [:cpeterson]]

Neither Chrome nor Safari appear to handle it.

Future

Find won't work in subframes without this fix.

Comment 5

[Neha Kochar [:neha]]

Not nightly blocking

Comment 6

[Neha Kochar [:neha]]

Discussed with Nika - for now, #2 looks fine. Neil, could we add a pref to turn off subframe arguments and see if it can ride the trains (with an intent-to-unship) ?

Comment 7

[Neil Deakin]

I can't seem to get the subframes argument to work any more. Regression checking points to the search subframes argument not working since 2019-july-05.

The range is https://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2019-07-06&enddate=2019-07-07 but I don't see anything obvious in or around that range.

At this point, I think adding a preference would be pointless.

Comment 8

[Neha Kochar [:neha]]

Thanks Neil! Then we should just send the unship email and remove the subframes argument from this API. This will make us consistent with other browsers and will also ensure we should not have an argument that doesn't actually work (since 2019).
Since this is broken even without Fission, I'm removing this from Fission queue.

Comment 9

[Neha Kochar [:neha]]

Changed the title of the bug to be more accurately descriptive of the work to be done.

Comment 10

[Chris Peterson [:cpeterson]]

Not a Fission bug