* For #21437 - Relocated Home-related settings to its dedicated sub screen
* For #21437 - Updated show top sites toggle text
* PR: Fixed lint warning. Reverted preference keys
* PR: added ignore for UI test
* PR: Added ignore for UI test
The bottom gray border of the header item from the Inactive Tabs section was correctly set when collapsing or expanding said section, but not on init. So if the section was initialized collapsed the gray border would not be present.
This regressed in our previous fix that made sure child tabs don't
mistakenly get moved out of the group if their parent is navigated
away, or in case the child tabs are redirected.
However, when a subsequent load occurs in any tab in the group the
search terms need to be cleared and the tab removed from the group
to prevent false positives.
* Fix description for `home_screen_displayed` metric
The current description appears to be incorrect by my reading of the source.
* Update app/metrics.yaml
Co-authored-by: Gabriel Luong <gabriel.luong@gmail.com>
Co-authored-by: Will Lachance <wlachance@mozilla.com>
Co-authored-by: Gabriel Luong <gabriel.luong@gmail.com>
* Docs only: Add extra context to `events.browser_menu_action`
This came up as a potential point of confusion in a discussion with DS. This is a docs only change.
* Update metrics.yaml
Co-authored-by: Will Lachance <wrlach@gmail.com>
* For #21360 - Added toggle for search term tab groups
* For #21360 - Lint cleanup
* PR: Added missing licenses and possibly fixed UI test
* PR: Added a "scrollTo" to potentially fix a UI test
* PR: Added potential fix for alwaysStartOnHomeTest
* PR: Added temporary ignore to alwaysStartOnHomeTest
* PR: added missing ignore comment
* For #21360 - Added missing feature flag driven visibility logic
Co-authored-by: Sebastian Kaspari <s.kaspari@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Home screen isn't actually visible in case we're displaying awesomebar
search results. The navigation is thus unnecessary and actually causes visual
jankiness as we display home for a moment before covering it up with
search results.
The latest geckoview-omni package correctly declares its capabilities,
including the `glean-native` one.
Additionally it is able to pick geckoview-omni over glean-native in all
configurations.
* Issue mozilla-mobile#21319 - Moved inactive tabs to the top of the normal tabs tray.
* Issue mozilla-mobile#21319 - Added a delete icon to delete ALL inactive tabs.
* Issue mozilla-mobile#21319 - Changed default inactive time period to 14 days
* Issue mozilla-mobile#21319 - Hooked inactive tabs setting to UI code
Inactive tabs setting is also disabled when the user has selected the one day or week auto-close tab setting.
* Issue mozilla-mobile#21319 - File and Lint cleanup
* PR: Fixed bug causing grouped tabs to also show in "Other" when marked as inactive but inactive is OFF in Settings
* PR: Fixed lint warnings
* PR: Removed redundant feature check
* PR - Ignore test until search term tab groups switch is done
These double comparisons are easier to read and see the pattern of on one line
so I'd rather keep them on one line. Additionally, it's difficult to
test each change individually so I'd rather not make additional changes.
To do this, I suppressed the max line length warning.
This commit was generated primarily by a macro that:
- appends `== component &&`
- appends `== item`
- (if applicable) Skips to the ending brace
- Go down one line and move cursor to the front of the line to prep for repeat
My only intervention was to skip extra lines to line it up to run again
and specify how many times in a row it should run.
---
The `to` in this code is an infix function that calls instantiates a
Pair under the hood. Subjectively observed, when this method is called
it generally hits the else case so 35 Pairs are instantiated each call -
that's 560 bytes. This method is called frequently - for example, an estimated
4 times each time a letter is typed on the homescreen and a measured 116 times
in a simple navigation (see the issue). The latter generates an estimated
63.4 KiB.
It was straightforward to remove these allocations so that's what this
change does.
The primary risk from this change is that it's difficult to test each
case to ensure it's working.
- stick to one naming scheme: rename articles to stories and use this all
throughout the app.
- add some spacing above the new section (as per the current design)
We currently have a 15s buffer to match metadata to its corresponding
visit. However, a existing metadata record can be updated more than
15s after it was created e.g. when closing the tab and updating
the view time.
We discovered that in a tab restore scenario we were recording view time
observations that were wrong - we'd record time deltas as-if user was
looking at the page while the browser wasn't running.
This happens because when we record a viewTime observation, we compare
current time with lastAccess time of the tab. In a restore scenario,
that lastAccess time happens to be from when the browser was last
running - which could be days ago.
The simplest solution was to not record a viewTime observation if the
url for a tab didn't change during a load event. To achieve this, we
needed to change which action we were using as a proxy for "navigation
events" - UpdateUrlAction contains the new url, allowing us to compare
against the current tab url.
Alternative solutions would be to keep using loading actions, but
dispatch a lastAccess event before performing a metadata update. This
would have worked, but would result in two lastAccess events being
dispatched for each navigation event instead of just one.
We only instrument onCreate because it's the only one with an
implementation.
While declaring this as a function that accepts a lambda is less
fragile, we've previously had issues with it such as suspected memory
leaks when used for telemetry. Therefore, we go with the simpler
approach.
This is a speculative fix for the intermittent issue. Typically, these
intermittents are caused by mocked lambdas but there is no mocked lambda
here. If this doesn't work, one `any()` argument fills in for a lambda:
it's possible that's causing the failure.
Unfortunately, I can't verify this fix easily because the "run test until
failure" option was removed from Android Studio.
See b3f5c87585 for a prior attempt to
address the intermittents in this class.
This is a bug we noticed after landing search term grouping.
An adapter can submit an empty list of items to the `ConcatAdapter`
early. This has the side-effect of triggering our `observeFirstInsert`
too soon and therefore updating the visibility to show the empty tray
placeholder and never switches back.
Our solution is to keep a constant observer on the adapter so we can
perform the visibility check on every insert/remove.
Co-authored-by: Roger Yang <royang@mozilla.com>
* For #21236: Separate tabs with the same search term into a different section in tabs tray
* Issue #21236: Scroll to selected tab + various tab fixes for groupings
* Issue #21236: Fix failing test
Co-authored-by: Jonathan Almeida <jalmeida@mozilla.com>
The test as it exists relies on the robolectric lifecycle, which is hard
to predict, so it doesn't seem worth fixing the test. Writing the test
any other way would require excessive mocking, which also seems
impractical.
The Glean core native code is now shipped through GeckoView directly
(through its `-omni` packages).
For local tests we need a library matching the host-platform, which is
available in the glean-native package.
* [WIP] New Layout for adding login and 'add login' button in 'SavedLoginsListView' to launch it.
Fixed bindings.
* [WIP] Removed "reveal password" button
* [WIP] Added interactor for the add login screen
* [WIP] Trying to check for duplicates
* [WIP] Renaming "addNew..." with "add..."
* [WIP] Check for duplicates
* [WIP] Fixes after merge
* Cleaning up the layout and making edit text for hostname selectable
* Error handling on add login screen. Tests for interactors and controllers
Co-authored-by: Vitaly V. Pinchuk <vetal.978@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This doesn't seem to be a high value test: increasing the view hierarchy
depth will only result in a performance problem on low end devices
if there is enough content on the new layer to cause the traversal to
take longer. It's more likely to result in a hard-to-workaround false
positive so we can remove it, like component init count.
This check doesn't seem high value because initializing a component
only indicates a performance problem if it's slow, which is not most
components, so it's likely to result in many false positives. To fix
the intermittent, we won't lose much if we remove it.
* Issue mozilla-mobile#21140 - Updated recent tab logic to show media tab and second-to-last tab, if the media tab was the last active tab.
* Fixed RecentTabsListFeatureTest unit test
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Compared with my local runs, CI sees +1 runBlockingIncrement calls so seems to
take other code paths. As such, we search for a range of rather than a single
value. The main downside I can think of is this could make the test trickier to
debug but we can wait and see if that's a problem in practice before taking
action on it.
This test wasn't running in CI
https://github.com/mozilla-mobile/fenix/issues/20386 so we changed the
CI configuration so that it will. However, the test was then failing so
this is the revision that unignores the test.
I wonder if componentInit count is useful - it seems like it'd cause
more false positives than not - but I figure we can leave it in and see
how it goes.
* Update Android Components version to 93.0.20210901143120.
* For #21043 - Integrate AC changes
* Fix breaking API changes of RestoreAction
Co-authored-by: Mugurell <Mugurell@users.noreply.github.com>
Co-authored-by: Christian Sadilek <christian.sadilek@gmail.com>
This is to investigate the intermittent mockk class
generation/loading issues. Since we can not reproduce
locally and the failures are intermittent they could
be caused by us running unit tests in parallel.
* Add telemetry probes for recent bookmarks on home screen. Tests for controller.
* Make the events into counters in the metrics ping
Update tests to reflect new metrics
Add data review link for new metrics
Mock new settings for startup metrics tests
Update metrics
Add test for recent bookmark glean events
* Recent bookmarks controller tests
Two new events are added:
- "inactive_tabs_expanded" for when the inactive tabs section is expanded
- "inactive_tabs_collapsed" for when the inactive tabs section is collapsed
For tracking when an inactive tab is opened / closed I've repurposed the
existing tabs tray telemetry (since the functionality uses the same code)
- tabs_tray.opened_existing_tab
- tabs_tray.closed_existing_tab
to support an extra "source" key indicating the feature from which a tab was
opened or closed. The current values for this new key are:
- "Tabs tray" for when a tab was opened/closed from tabs tray
- "Inactive tabs" for when a tab was openes/closed from the Inactive tabs
section of the tabs tray.
This means no code will be generated by Kotlin Android Extensions for caching
views and also for @Parcelize annotated classes.
As recommended in the official documentation
https://developer.android.com/topic/libraries/view-binding/migration#gradle
we need to switch on using `kotlinx.parcelize.Parcelize` instead of
`import kotlinx.android.parcel.Parcelize`
For #17917 - Remove the `kotlin-android-extensions` plugin
This means no code will be generated by Kotlin Android Extensions for caching
views and also for @Parcelize annotated classes.
As recommended in the official documentation
https://developer.android.com/topic/libraries/view-binding/migration#gradle
we need to switch on using `kotlinx.parcelize.Parcelize` instead of
`import kotlinx.android.parcel.Parcelize`
This ensures that we do not attempt places initialization before
Megazord.init() finishes, and that touching BrowserStore does not
kick-off places initialization (due to the `cleanup` call being a
side-effect of creating metadata middleware, used as part of the
BrowserStore).
Glean initialization happens before initialize megazord, and it touches
core.store BrowserStore instance, kicking-off places initialization on
an IO thread (due to the cleanup call), which raced megazord
initialization on the main thread.
App init sequence is a bit of a mine-field, so this patch takes the easy
way out and doesn't attempt to re-order initialization sequence. Also,
initializing places as a side-effect of touching BrowserStore was also,
clearly, a bug.
Noah Bond
Roger Yang
Sebastian Kaspari
Gabriel Luong
Arturo Mejia
codrut.topliceanu
AndiAJ
Mugurell
Jonathan Almeida
Christian Sadilek
Elise Richards
Biren-Nayak
Oana Horvath
Mozilla L10n Automation Bot
Grisha Kruglov
Jan-Erik Rediger
Aaron Train
Michael Comella
Andrei Joltan
Travis Long
rayanHappy
rxu
Rohan Maity
bendk
Noah Bond
mcarare
czlucius
Mickey Moz