Status Update
No update yet.
Comments
vi...@google.com
Thanks for the detailed analysis and sample. I've prepared a CL, but I'd also be happy to review an external contribution since you already went through the trouble of identifying a fix. Just let me know here.
vi...@google.com
If you have a CL ready go for it.
That being said, I just went down git history and found when this was introduced:
As you can see, this was introduced to fix a "memory leak" though I don't quite understand why creating a new callback wrapper for every setSupportActionBar() call is a problem.
li...@gmail.com
Branch: androidx-main
commit 200ccecbc8c7c5ba555e7fa15890edff67affb58
Author: Alan Viverette <alanv@google.com>
Date: Fri Apr 30 14:55:07 2021
Fix case where setSupportActionBar overwrites the window callback
RelNote: """Fix scenario where calling setSupportActionBar after setting the
window callback would overwrite the callback."""
Test: SupportActionBarTestCase
Fixes: 186791590
Change-Id: Ie43eedf8322caa44e7b201a95cbc64197953e020
A appcompat/appcompat/src/androidTest/java/androidx/appcompat/app/SupportActionBarTestCase.kt
M appcompat/appcompat/src/main/java/androidx/appcompat/app/AppCompatDelegateImpl.java
vi...@google.com
ki...@google.com
I vaguely remember this. The issue was iirc:
- You use multiple fragments (say switched via tabs). Each fragment calls
setSupportActionBar()when visible. - The old code would wrap the current
Window.Callbackwith something fromToolbarActionBar(TAB), then set it as the window callback. - When the user switches to Tab #2, we'd do the same thing, but this time the callback wrapper chain would be
(((Root callback) TBAB1Wrapper) TBAB2Wrapper). TBAB1Wrapper would keep a reference a reference to the views even though the #1 fragment has likely been destroyed.
I've just +2'd Alan's CL, but the proper fix is properly to unwrap the callback from Window.getCallback().
Description
Version used: 1.1.0-alpha04
Theme used: Theme.MaterialComponents.DayNight.NoActionBar
Devices/Android versions reproduced on: android pie
- When I set toolbar_maxButtonHeight, since buttonGravity can only be set to top and bottom, it does not meet the CENTER_VERTICAL requirements of the design.