Repository navigation
Fix the app menu entry for Chrome v154 - #310
Closed
matheuslive wants to merge 1 commit into
Closed
matheuslive wants to merge 1 commit into
matheuslive wants to merge 1 commit into
Conversation
M154 reworked PropertyModel: values are no longer boxed in a one field ValueContainer but stored as they are, with a shared static sentinel marking keys never written, and the PropertyModel(List) constructor is gone in favour of the varargs PropertyModel(PropertyKey...) one. Reading the first field of each value returned garbage, so no row looked like a standard menu item and the ChromeXt entries were dropped with "Cannot build a standard app menu item". Detect the format through the sentinel, unbox only the old containers, build the model through whichever key constructor exists, and write the page info overrides straight into the map when there is no container. Closes JingMatrix#308
Author
|
Also tested on Chrome Beta 156.0.8078.4 (Android 13): the "Eruda console" entry shows up in the app menu and opens Eruda, with no ChromeXt exceptions in the log. |
|
Any chance of generating a build for this so it can be installed? |
Manually trigger the workflow. I just ran a build: https://github.com/SainoNamkho/ChromeXt/actions/runs/38844598262 |
Owner
|
Thanks for contribution, I believe it is solved in the merged (updated) pr #306. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This builds on #306 and fixes the remaining menu failure reported in #308. On Chrome 154.0.8037.126 the hooks install and userscripts run with the
chromium-compatbranch, but the app menu entry is dropped withCannot build a standard app menu item for the ChromeXt entries.The cause is a rework of
org.chromium.ui.modelutil.PropertyModelin M154:ValueContainer. The map holds the values themselves, and a key that was never written points to a shared sentinel,public static final ObjectonPropertyModel. Reading the first declared field of each value therefore returned garbage: no row had aStringtitle, so no standard template was found, andmenuIdNameOfreturnednullfor every row.PropertyModel(List<PropertyKey>)constructor is gone. Only(Map, int)and the varargs(PropertyKey...)remain, somodelOfKeyswasnull.(key, value)setters are unchanged.The fix tells the two formats apart by the presence of that static
Objectfield. It unboxes only the old containers and builds the model through whichever key constructor exists, passing a typedPropertyKey[]to the varargs one. The page info overrides are written straight into the map when there is no container. The old path keeps its previous behaviour.I tested it on a Samsung S20 FE (Android 13) with Chrome 154.0.8037.126. The Eruda console entry shows up after the third divider, and tapping it opens Eruda on the page. On 154 the icon row at the top of the menu no longer carries a page info entry, so the reader mode override simply finds nothing there and logs nothing. I have not re-tested older Chrome versions with this change.
Closes #308