Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

First page load calls onLoadingStateChanged with loading=false twice #3676

Closed
boek opened this issue Jun 26, 2019 · 4 comments
Closed

First page load calls onLoadingStateChanged with loading=false twice #3676

boek opened this issue Jun 26, 2019 · 4 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. E2 Estimation Point: easy, half a day to 2 days eng:qa:verified QA Verified Feature:Telemetry P2 Upcoming release

Comments

@boek
Copy link
Contributor

boek commented Jun 26, 2019

QA noticed that there was a discrepancy between uri_count and search_count and after some further investigation it looks like onLoadingStateChanged from the session observer is getting called twice on the first load in a session.

This is causing an extra Event.UriOpened event to get tracked.

┆Issue is synchronized with this Jira Task

@boek boek added 🐞 bug Crashes, Something isn't working, .. Feature:Telemetry P2 Upcoming release labels Jun 26, 2019
@vesta0 vesta0 added this to the Bugs milestone Jul 3, 2019
@vesta0 vesta0 removed this from the Bugs and Polish milestone Jul 24, 2019
@mcomella
Copy link
Contributor

mcomella commented Oct 8, 2019

This could be a good candidate for a regression unit test! 😁

@sblatz sblatz self-assigned this Oct 10, 2019
@sblatz sblatz removed their assignment Oct 10, 2019
@severinrudie severinrudie self-assigned this Oct 18, 2019
@severinrudie
Copy link
Contributor

I also noticed #6126 while investigating this bug, which probably accounts for some of the discrepancy. Note that #6126 only impacts uri_count, not search_count.

@severinrudie
Copy link
Contributor

Verified this behavior on Reference Browser at mozilla-mobile/reference-browser#926 and opened mozilla-mobile/android-components#4795 against AC. Opening a PR with a temporary fix from our side.

severinrudie added a commit to severinrudie/fenix that referenced this issue Oct 18, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@severinrudie severinrudie added the E2 Estimation Point: easy, half a day to 2 days label Oct 19, 2019
boek pushed a commit that referenced this issue Oct 23, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@boek boek added the eng:qa:needed QA Needed label Oct 23, 2019
@AndiAJ AndiAJ added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Oct 24, 2019
@AndiAJ
Copy link
Collaborator

AndiAJ commented Oct 24, 2019

Hi, verified as fixed on Nightly Build #12970607 from 24/10.

Ping 463e6ecc-0b48-4355-92b9-af151f6ac7e9 - 3 searches performed ✔️
Ping d523cd31-b93e-4efd-aa41-aef8ed8ae956 - 5 searches performed ✔️
Ping 5ccb0c78-4401-47ee-9777-0542d52840a4 - 8 searches performed ✔️

Logcat

Ping 0fc0f19c-15af-4502-9d16-868968b283a5 - 6 searches performed, (one with each search engine) ✔️
Logcat

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. E2 Estimation Point: easy, half a day to 2 days eng:qa:verified QA Verified Feature:Telemetry P2 Upcoming release
Projects
None yet
Development

No branches or pull requests

6 participants