Have AppCompat Toolbar call onPrepareMenu() at the correct time [232206677]

Fixed

Bug

Status Update

No update yet.

Description

to...@yahoo.com

created issue #1

May 11, 2022 04:59PM

Please see https://issuetracker.google.com/227376894

and the earlier related issue(s)

This needs to be re-opened as the fix mentioned in

https://issuetracker.google.com/issues/227376894#comment3

is itself introducing a new bug; please the source code for that fix.

AppCompat 1.5.0-alpha01.

Please may I remind you about the contract for onPrepareMenu: called when the menu is first shown

But in your code, you now simply call onPrepareMenu during the onCreateMenu phase.

I realise that Toolbar did/does not implement onPrepareMenu properly up to now, but it never promised to do, so that was ok up to now.

However, if you're adding support for MenuProvider (thank you!) then it has to be implemented such that it's contract is fulfilled. Right now, the contract of the MenuProvider is broken in Toolbar.

Comments

ow...@google.com <ow...@google.com> May 20, 2022 01:28PM

Assigned to al...@google.com.

al...@google.com <al...@google.com> May 20, 2022 01:53PM

Reassigned to sa...@google.com.

il...@google.com <il...@google.com> Jun 16, 2022 09:37PM

Reassigned to jb...@google.com.

jb...@google.com <jb...@google.com> #2Jun 30, 2022 12:43AM

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

to...@yahoo.com <to...@yahoo.com> #3Jun 30, 2022 10:26AM

reuploading my previous sample app.

It's fairly simple.

Call order with AppCompat 1.5.0-alpha01:

onCreateMenu
onPrepareMenu
user opens menu
menu is displayed

According to the contract:

"Called by the MenuHost right before the Menu is shown.

it should be:

onCreateMenu
user opens menu
onPrepareMenu
menu is displayed

build and run the sample. It has examples for both activity and fragment. Follow the print out's in the log.

MenuProv.zip

612 KB

Download

ap...@google.com <ap...@google.com> #4Aug 3, 2022 09:07PM

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

commit 723f2ef10ea1de61e94de16b917cc6e939bfc211
Author: Jeremy Woods <jbwoods@google.com>
Date: Tue Aug 02 17:05:42 2022

Fix Toolbar onPrepareMenu timing with MenuHost

Instead of unconditionally calling onPrepareMenu() when Toolbar executes
onCreateMenu(), it should wait until the menu is about to be shown to
make the call.

This will ensure that Toolbar properly fulfills the MenuHost contract.

RelNote: "Toolbar will now call `onPrepareMenu()` before the menu is shown
instead of unconditionally when the menu is created."
Test: modified the ToolbarMenuHostTest
Bug: 232206677

Change-Id: I2a58df0064c08b9ae87962a41d87e7bc60073df0

M appcompat/appcompat/src/main/java/androidx/appcompat/widget/Toolbar.java
M appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/ToolbarMenuHostTest.kt

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

jb...@google.com <jb...@google.com> #5Aug 3, 2022 09:10PM

Marked as fixed.

This has been fixed internally and will be available in the AppCompat 1.6.0-beta01 release.

ju...@gmail.com <ju...@gmail.com> #6Jan 26, 2023 01:12AM

Me hakearon y me robaron mi privacidad eyos se encuentran en amarillo tz

yu...@gmail.com <yu...@gmail.com> #7Feb 9, 2024 11:23PM

Delete all old