Incorrect font weight are set for DownloadableFonts on API 29+ [194553426]

Fixed

Bug

Status Update

No update yet.

Description

ri...@gmail.com

created issue #1

Jul 24, 2021 01:22PM

Artifact used: androidx.core:core:1.6.0 Version used: 1.6.0 Theme used: (Not related) Devices/Android versions reproduced on: Pixel 5 Android 11

https://cs.android.com/androidx/platform/frameworks/support/+/c32cc6c885b20be94d2d1c0498e4a978161d3f61:core/core/src/main/java/androidx/core/graphics/TypefaceCompatApi29Impl.java;l=131-136

final FontStyle defaultStyle = new FontStyle(
    (style & Typeface.BOLD) != 0 ? FontStyle.FONT_WEIGHT_BOLD
       : FontStyle.FONT_WEIGHT_NORMAL,
    (style & Typeface.ITALIC) != 0 ? FontStyle.FONT_SLANT_ITALIC
       : FontStyle.FONT_SLANT_UPRIGHT
);

return new Typeface.CustomFallbackBuilder(familyBuilder.build())
    .setStyle(defaultStyle)
    .build();

This part will make the final Typeface lost the correct font-weight from the FontInfo. For example, request for a font with 500 weight, but the final Typeface has 400 weight. This will make the fallback font incorrectly displayed.

The fix is to use the style from the best-matched font, just like the platform, https://cs.android.com/android/platform/superproject/+/android-11.0.0_r1:frameworks/base/graphics/java/android/graphics/Typeface.java;l=276-285.

Comments

ow...@google.com <ow...@google.com> Aug 2, 2021 01:07PM

Assigned to no...@google.com.

ri...@gmail.com <ri...@gmail.com> #2Aug 2, 2021 06:32PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit d92e6ab2da02250bfbff957f733a4a2aaaf0c82c
Author: Ian Lake <ilake@google.com>
Date: Thu Jul 09 16:04:11 2020

Allow multiple destinations in NavDeepLinkBuilder

Expand the ability of NavDeepLinkBuilder to support
adding multiple destinations. This allows developers
to effectively build a custom synthetic back stack
that goes through multiple destinations in the same
graph or through destinations in separate graphs.

This augments, not replaces, the existing synthetic
back stack logic: the start destinations of any
graphs needed to make the chosen destination visible
*are* still added to the back stack.

To maintain behavior compatibility, setArguments()
continues to apply the arguments set there to all
destinations created by the deep link. However, this
CL would be first step to also supporting arguments
at a destination level.

Test: newly added tests pass
Relnote: "`NavDeepLinkBuilder` now supports adding
multiple distinct destinations to the generated
back stack."
BUG: 147913689

Change-Id: I3ee0d5251ec1047774aa4e826b25a6d8cf4ec28d

M navigation/navigation-common/src/main/java/androidx/navigation/NavDestination.java
M navigation/navigation-common/src/test/java/androidx/navigation/NavDestinationTest.kt
M navigation/navigation-runtime/api/2.4.0-alpha01.txt
M navigation/navigation-runtime/api/current.txt
M navigation/navigation-runtime/api/public_plus_experimental_2.4.0-alpha01.txt
M navigation/navigation-runtime/api/public_plus_experimental_current.txt
M navigation/navigation-runtime/api/restricted_2.4.0-alpha01.txt
M navigation/navigation-runtime/api/restricted_current.txt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.java
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavDeepLinkBuilder.java

https://android-review.googlesource.com/1359698

se...@google.com <se...@google.com> #3Mar 29, 2022 06:29PM

Reassigned to se...@google.com.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit cd8fc14c6f2ad77d563e726eb30aa36f324259d0
Author: Ian Lake <ilake@google.com>
Date: Fri Jul 10 13:22:43 2020

Support per destination arguments in NavDeepLinkBuilder

In addition to continuing to support setArguments()
as a way to set global arguments that apply to all
destinations, add an optional Bundle parameter to
setDestination() and addDestination() to allow per
destination arguments.

Test: updated tests pass
BUG: 147913689
Relnote: "`NavDeepLinkBuilder` now allows you to set
arguments at a per destination level in addition to
the global arguments set via `setArguments()`."

Change-Id: I3a18cbb67b5836db013804a6d6c468cb0bc22102

M navigation/navigation-runtime/api/2.4.0-alpha01.txt
M navigation/navigation-runtime/api/current.txt
M navigation/navigation-runtime/api/public_plus_experimental_2.4.0-alpha01.txt
M navigation/navigation-runtime/api/public_plus_experimental_current.txt
M navigation/navigation-runtime/api/restricted_2.4.0-alpha01.txt
M navigation/navigation-runtime/api/restricted_current.txt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavDeepLinkBuilderTest.kt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.java
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavDeepLinkBuilder.java

https://android-review.googlesource.com/1360905

ap...@google.com <ap...@google.com> #4Mar 30, 2022 08:38AM

Would it be possible to add a NavDirections overload to NavDeepLinkBuilder for this? It has both the id and the args so it's a nice way to add destination. I can also create an extra ticket.

lo...@gmail.com <lo...@gmail.com> #5Mar 30, 2022 08:59AM

Currently deep links always start with the nav_graph's startDestination. This is a problem for apps which use multiple root-level destinations in a bottom-navigation view, and unexpected for explicitly created synthetic task stacks using only addDestination().

I'm using an invocation like: navDeeplinkBuilder.addDestinaton(frag_two).addDestination(frag_three), where start_destination = frag_one. Using an initial setDestination doesn't seem to change anything. frag_one is always at the bottom of the back stack, with two and three above.

After building the deep link, I can see the first R.id is always 0 (or the android:id of the nav_graph, if you remembered to set one), as a result of NavDeepLinkBuilder's node.buildDeepLinkIds(prevdestination)

When navController receives the deep link, it will attempt to navigate to 0 (the navGraph itself), which NavGraphNavigator always substitutes with the startDestination.

Separately, the navGraph will be explicitly added above it as the parent of that startDestination using the mGraph id set on the end Fragment. link

So it seems like both NavDeepLinkBuilder and NavController are attempting to set the mGraph to the bottom of the task stack. I think the initial navGraph should just be implicit in the NavDeepLinkBuilder-created Intent, since there's already a stated dependency on the navGraph being the same at Intent-build-time and when the deep link is executed.

se...@google.com <se...@google.com> #6Mar 30, 2022 03:56PM

Project: platform/frameworks/support
Branch: androidx-main

commit 64a958a72265aa72aae6dea6e78d531d0d5be096
Author: Faelyn O'Grady <fogrady@google.com>
Date: Wed Jul 21 13:20:11 2021

Fix deep link ids from a navGraph's id to one of its siblings.

For a nested NavGraph N with startDest=A, and another dest B in N:
addDestination(N)
addDestination(B)

Would previously result in:
[N, N, B]

This change fixes the result to:
[N, B]

(which implies [A, B], since A is the startDest)

Because B's buildDeepLinkIds(prevDestination = N) would generate both [N, B], even though N was already navigated to (literally set as the previousDestination).

Test: New unit tests included. All existing tests pass.
BUG: 147913689
Change-Id: I42da530ebb72be1a90164416b7ca7f95e7834bba

M navigation/navigation-common/src/main/java/androidx/navigation/NavDestination.kt
M navigation/navigation-common/src/test/java/androidx/navigation/NavDestinationTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavDeepLinkBuilderTest.kt
A navigation/navigation-runtime/src/androidTest/res/navigation/nav_non_start_nest.xml

https://android-review.googlesource.com/1773621

se...@google.com <se...@google.com> Jun 29, 2022 04:41PM

Marked as fixed.