Status Update
Comments
il...@google.com <il...@google.com> #2
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
vs...@gmail.com <vs...@gmail.com> #3
Unfortunately I can't. I can add something to that if you'd hint me kindly what additional info might help.
il...@google.com <il...@google.com>
jd...@gmail.com <jd...@gmail.com> #4
The version comparison insdie ActivityResultFragmentVersionDetector is clearly inadequate. The simple length and string comparison end up returning true when they shouldn't
From the lint check:
val currentVersion = library.substringAfter("androidx.fragment:fragment:")
if (library != currentVersion && FRAGMENT_VERSION.compareVersions(currentVersion)) {
locations.forEach { location ->
context.report(
ISSUE, expression, location,
"Upgrade Fragment version to at least $FRAGMENT_VERSION."
)
}
}
But if you plugin the the current version of "1.3.1", you can see how it yields the wrong result.
val FRAGMENT_VERSION = "1.3.0"
fun String.compareVersions(other: String): Boolean {
return when {
length < other.length -> true
length > other.length -> false
else -> this < other
}
}
FRAGMENT_VERSION.compareVersions("1.3.1")
res3: kotlin.Boolean = true
zh...@google.com <zh...@google.com> #5
I'm hitting this bug as well when fragment-ktx
is upgraded from 1.3.0
to 1.3.1
.
Looking at compareVersions()
function in it:
const val FRAGMENT_VERSION = "1.3.0"
...
if (library.isNotEmpty()) {
val currentVersion = library.substringAfter("androidx.fragment:fragment:")
if (library != currentVersion && FRAGMENT_VERSION.compareVersions(currentVersion)) {
locations.forEach { location ->
context.report(
ISSUE, expression, location,
"Upgrade Fragment version to at least $FRAGMENT_VERSION."
)
}
}
}
...
private fun String.compareVersions(other: String): Boolean {
return when {
length < other.length -> true
length > other.length -> false
else -> this < other
}
}
So say library
is androidx.fragment:fragment:1.3.1
, currentVersion
would be 1.3.1
and the Lint detector reports an issue if "1.3.0".compareVersions("1.3.1")
returns true
. And since "1.3.0"
and "1.3.1"
has the same length, it reports an issue if "1.3.0" < "1.3.1"
, which is true
(
I'm confused about how the current compareVersions()
implementation is supposed to work for semantic versions, because it doesn't seem to work for a number of scenarios I could think of:
Case | FRAGMENT_VERSION |
currentVersion |
length <=> other.length |
this < other |
compareVersions() (reports Lint error) |
Works as expected |
---|---|---|---|---|---|---|
Older stable release | "1.3.0" |
"1.2.0" |
length == other.length |
false |
false |
false |
Older unstable release | "1.3.0" |
"1.3.0-alpha01" |
length < other.length |
N/A | true |
true |
Exact unstable release (aosp/1564487) | "1.3.0" |
"1.3.0-alpha06" |
length < other.length |
N/A | true |
false |
Newer unstable release | "1.3.0" |
"1.3.0-beta01" |
length < other.length |
N/A | true |
false |
Exact stable release | "1.3.0" |
"1.3.0" |
length == other.length |
false |
false |
true |
Newer stable release | "1.3.0" |
"1.3.1" |
length == other.length |
true |
true |
false |
zh...@google.com <zh...@google.com> #6
Oh, if the intention here is to simply report an issue for all non-stable versions (which will have a different string length) to avoid pulling in a full semver implementation, I think we just need to flip the comparison in else -> this < other
here to fix this for stable versions.
jd...@gmail.com <jd...@gmail.com> #7
Flipping it would work for a quick fix, but the length check may still end up returning incorrect data if the patch version of fragment ever reached 2 digits, 1.3.10.
ap...@google.com <ap...@google.com> #8
Branch: androidx-main
commit 2db691788fe04d6cb2080cd2187989962a2c2b46
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Mar 10 15:18:15 2021
Fix string comparision for activity lint
Fixing error that caused lint report when using Fragment versions
greater than 1.3.0.
RelNote: "There is no longer a false positive lint error when using
Fragment version 1.3.1 with Activity Result APIs"
Test: ActivityResultFragmentVersionDetectorTest
Bug: 182388985
Change-Id: I54da158c0ca4bfa6246f51226fd4991dc485b0d2
M activity/activity-lint/src/main/java/androidx/activity/lint/ActivityResultFragmentVersionDetector.kt
M activity/activity-lint/src/test/java/androidx/activity/lint/ActivityResultFragmentVersionDetectorTest.kt
jb...@google.com <jb...@google.com> #9
This has been fixed internally and will be available in the next Activity release.
vs...@gmail.com <vs...@gmail.com> #10
Awesome, thank you!
ba...@gmail.com <ba...@gmail.com> #11
to...@yahoo.com <to...@yahoo.com> #12
[Deleted User] <[Deleted User]> #13
ce...@gmail.com <ce...@gmail.com> #14
Release notes of
We still get it with Activity 1.2.2 and Fragment 1.3.2.
to...@yahoo.com <to...@yahoo.com> #15
implementation 'androidx.fragment:fragment:1.3.2'
finally seems to work...
ce...@gmail.com <ce...@gmail.com> #16
Edit for
The issue is fixed.
I'm sorry for the previous message, the issue was caused by the dependency configuration of our project (some modules had a transitive dependency on previous activity/fragment versions).
lb...@gmail.com <lb...@gmail.com> #17
il...@google.com <il...@google.com> #18
Upgrading to Activity 1.2.2 (or Activity 1.3.0-alpha05, which also includes the fix) is the only requirement.
Fragment 1.3.2 includes a dependency on Activity 1.2.2, so upgrading your version of Fragments will also pull in the fix via the transitive dependency.
lb...@gmail.com <lb...@gmail.com> #19
implementation "androidx.fragment:fragment-ktx:1.3.2"
?
But how do you guys know this fixed the issue?
lb...@gmail.com <lb...@gmail.com> #21
ca...@gmail.com <ca...@gmail.com> #22
```
Update to Fragment 1.3.0 to use ActivityResult APIs
```
il...@google.com <il...@google.com> #23
Re #22 - please file a new bug with a sample project that reproduces your issue.
ar...@gmail.com <ar...@gmail.com> #24
ar...@gmail.com <ar...@gmail.com> #25
ar...@gmail.com <ar...@gmail.com> #26
du...@gmail.com <du...@gmail.com> #27
il...@google.com <il...@google.com> #28
Re #24-27 - please file a new issue with a project that reproduces your issue.
mu...@gmail.com <mu...@gmail.com> #29
mu...@gmail.com <mu...@gmail.com> #30
ig...@gmail.com <ig...@gmail.com> #31
implementation "androidx.activity:activity-ktx:
implementation "androidx.fragment:fragment-ktx:1.3.4"
il...@google.com <il...@google.com> #32
Re #29, #31 - like previously mentioned, file a new issue with a project that reproduces your issue.
si...@gmail.com <si...@gmail.com> #33
df...@smart-soft.com <df...@smart-soft.com> #34
il...@google.com <il...@google.com> #35
Re
Description
Component used: Activity
Version used: 1.3.0-alpha04
InvalidFragmentVersionForActivityResult
is reported though I forcedandroidx.fragment:fragment-ktx:1.3.1
.Here is my build.gradle essentials: