[1.7.0] WindowInsetsControllerCompat has different behavior depending on method of creation on API 30 [207401542]

Fixed

Bug

adexe s nau

Status Update

No update yet.

Description

va...@google.com

created issue #1

Nov 22, 2021 08:45PM

This is effectively a follow-up to b/180881870

The fix to that bug (https://android-review.googlesource.com/1783607) stored the window parameter from one of the constructors to Impl30.

However, this was not the primary constructor, which means that only instances of WindowInsetsControllerCompat constructed with a Window will be able to workaround b/180881870 on API 30.

In particular, the code-path that uses WindowInsetsControllerCompat.toWindowInsetsControllerCompat(insetsController: InsetsController) bypasses the window constructor, meaning that the bug will still occur on API 30 with any method of retrieving the WindowInsetsControllerCompat with that code-path.

Those methods include:

ViewCompat.getWindowInsetsController(view: View)
WindowCompat.getInsetsController(window: Window, view: View)
WindowInsetsControllerCompat.toWindowInsetsControllerCompat(insetsController: InsetsController)

The current workaround is to avoid all of the above methods, and instead directly use the proper constructor: WindowInsetsControllerCompat(window: Window, view: View).

As a proposed fix, the constructor of WindowInsetsControllerCompat that doesn't take a Window should be deprecated for API 30, since the Window is required to workaround b/180881870. This would necessarily bubble up to the other methods, ensuring that the bug will be fixed across all versions.

Comments

al...@google.com <al...@google.com> Nov 22, 2021 08:46PM

Reassigned to ca...@google.com.

ro...@google.com <ro...@google.com> #2Dec 10, 2021 08:26PM

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

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

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

Message last modified on Dec 10, 2021 08:26PM

va...@google.com <va...@google.com> #3Dec 10, 2021 10:10PM

Ah, interesting.

So if we remove the dependency on Window altogether because we can shift the implementation away from using the Window, then we'd want all code paths for creating a WindowInsetsControllerCompat to take just a View.

Effectively, everything should just go through ViewCompat.getWindowInsetsController.

The internal implementation then would be able to make the same workarounds (setting setSystemUiVisibility, etc.) using the View reference instead of the Window reference.

The most awkward case is still the WindowInsetsControllerCompat.toWindowInsetsControllerCompat method, which we would want to take the View (and therefore makes it equivalent to the ViewCompat.getWindowInsetsController)

Does that sound right?

Message last modified on Dec 10, 2021 10:11PM

ca...@google.com <ca...@google.com> #4Dec 13, 2021 09:50AM

Reassigned to va...@google.com.

That sounds good to me. I think it always good if developers know where to look for to get a Compat instance even though it takes one more parameters. And I think it's fine to delegate the call of WindowInsetsControllerCompat.toWindowInsetsControllerCompat to ViewCompat.getWindowInsetsController

I'll reassign this bug to you since you've done a lot of work on it already, but feel free to reassign it back if needed.

ap...@google.com <ap...@google.com> #5Jan 18, 2022 04:31PM

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

commit 0d8eb4081585d1be1997cb097f5fd54b41ddf8bb
Author: Alex Vanyo <vanyo@google.com>
Date: Tue Nov 23 01:06:28 2021

Convert WindowInsetsControllerCompat behavior to depend on a View to align behavior

Relnote: Converted WindowInsetsControllerCompat to depend on a View instead of a Window or the platform WindowInsetsController. This improves behavior with Dialogs, showing the IME, and ensures the non-deprecated creation methods workaround

https://issuetracker.google.com/issues/180881870.

Bug: 207401542,210121779
Test: Updated and new tests

Change-Id: I292640cb4acc39e47a4d2ce0502293d42468ed67

M core/core/api/restricted_current.txt
M core/core/src/androidTest/java/androidx/core/view/WindowInsetsAnimationCompatActivityTest.kt
M core/core/src/main/java/androidx/core/view/ViewCompat.java
M core/core/api/public_plus_experimental_current.txt
M samples/Support4Demos/src/main/java/com/example/android/supportv4/view/WindowInsetsControllerPlayground.kt
M core/core/src/main/java/androidx/core/view/WindowCompat.java
M core/core/src/androidTest/java/androidx/core/view/WindowInsetsControllerCompatActivityTest.kt
M core/core/api/current.txt
M core/core/src/main/java/androidx/core/view/WindowInsetsControllerCompat.java
M core/core/src/androidTest/java/androidx/core/view/WindowInsetsCompatActivityTest.kt
M core/core/src/androidTest/java/androidx/core/view/ViewCompatTest.java

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

va...@google.com <va...@google.com> Jan 18, 2022 06:15PM

Marked as fixed.

Issue 207401542