Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Erratic results of find in page #4704

Closed
csadilek opened this issue Oct 9, 2019 · 8 comments
Closed

Erratic results of find in page #4704

csadilek opened this issue Oct 9, 2019 · 8 comments
Labels
🐞 bug Something isn't working <findinpage> shows a UI for results of "find in page" needs:gv To implement/fix this we need a new API in GeckoView

Comments

@csadilek
Copy link
Contributor

csadilek commented Oct 9, 2019

STR:

  • Load a page (cnn.com works well for reproducing)
  • Step 1: Type 'a', then type 'n'
  • Step 2: Hit ^ to display next result a few times in a row (quickly)

Expected after Step 1:
We'd expect the result counts of 'a' then the result counts of 'n'.

Actual after Step 1:
Sometimes after typing 'n' we still show the result counts of 'a'.

Expected after Step 2:
Iterating through the results causes the counter to increment/decrement and the total results to stay stable.

Actual after Step 2:
Sometimes we display 0/0, sometimes the total results change.

The problem of the incorrect result count in step 2, I can also reproduce in Fennec, but it definitely works much better there.

┆Issue is synchronized with this Jira Task

@csadilek csadilek added the 🐞 bug Something isn't working label Oct 9, 2019
@csadilek csadilek added the <findinpage> shows a UI for results of "find in page" label Oct 10, 2019
@csadilek
Copy link
Contributor Author

Tested this with latest R-B and Fenix release builds.

@csadilek
Copy link
Contributor Author

csadilek commented Oct 10, 2019

UPDATE: This looks like it was also a problem before our recent refactoring. (I am actually not sure about that.) Looking at our Flow/Reducer logic again, it looks like we should always forward the last result correctly.

One theory:
When calling findAll in quick succession, there's no guarantee that the results come back in order. So, while we always display the last result, it doesn't mean that the last result is from the last search (it could be from a previous input character). Then we'd need a similar mechanism to our awesomebar where we either cancel previous searches or discard stale results. Wdyt @pocmo @Amejia481 ?

@pocmo
Copy link
Contributor

pocmo commented Oct 10, 2019

Yeah, that makes totally sense.

If we'd already use the Store from the engine I'd say we could store the search string in the state and then in the reducer only update when the result is for the same search string.

Maybe we could emulate this? We already pass the text to EngineObserver.onFind() but we only use it to clear the list. If we keep it then we can use that once EngineObserver.onFindResult() gets called (onFindResult() would need to tell what text it is for though!).

@csadilek
Copy link
Contributor Author

csadilek commented Oct 10, 2019

@pocmo after some more testing, I think this is (also) a GV bug. The GeckoSession.FinderResults contain the search string and I see those results come back in the right order (with the right search string), but with inconsistent values. I also see interleaved 0/0 results come back on find next (when called in quick succession) which are incorrect too.

So, I've verified that even when things happen in the right order the results we get back can be incorrect. I'll file an issue.

@csadilek
Copy link
Contributor Author

@pocmo
Copy link
Contributor

pocmo commented Feb 11, 2020

@data-sync-user data-sync-user changed the title Erratic results of find in page FNX3-22489 ⁃ Erratic results of find in page Aug 2, 2020
@data-sync-user data-sync-user changed the title FNX3-22489 ⁃ Erratic results of find in page FNX2-16990 ⁃ Erratic results of find in page Aug 3, 2020
@st3fan st3fan changed the title FNX2-16990 ⁃ Erratic results of find in page Erratic results of find in page Aug 5, 2020
@pocmo
Copy link
Contributor

pocmo commented Oct 4, 2021

Focus: mozilla-mobile/focus-android#5181

@csadilek
Copy link
Contributor Author

Closing in favour of https://bugzilla.mozilla.org/show_bug.cgi?id=1587856.

Fenix: GeckoView automation moved this from Backlog to Done Sep 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working <findinpage> shows a UI for results of "find in page" needs:gv To implement/fix this we need a new API in GeckoView
Projects
No open projects
Development

No branches or pull requests

2 participants