Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable finding of undisplayed menu items #827

Open
wants to merge 1 commit into
base: 0_7_x
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 19 additions & 1 deletion pywinauto/controls/uia_controls.py
Original file line number Diff line number Diff line change
Expand Up @@ -938,9 +938,27 @@ def __init__(self, elem):
super(MenuItemWrapper, self).__init__(elem)

# -----------------------------------------------------------
_cache_items = {}
def items(self):
"""Find all items of the menu item"""
return self.children(control_type="MenuItem")
items = self.children(control_type="MenuItem")

# If a menu is not displayed then the UIA controls (children of self)
# will not yet exist. We can bring them into existence by clicking the
# menu (self). The controls can also (mysteriously) lose all their
# element_info. So once we see them bv clicking the menu we cache them
# for subsequent use.
if not items:
if self._cache_items.get(self.element_info.automation_id, None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't rely on automation_id property as a key of dictionary because it's possible to have child items without such property. I can't provide an example right now because of being too busy, but it's worth trying different apps with this approach to test everything works the same way or better.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, time flies. I too am only looking at this again now. If automation_id is not reliable we could just check that it is available and if it is, then cache the item.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never say "Never!". :) Yeah, that may be better, but why not use self.element_info.name? Maybe it's a specific menu implementation. Is it Qt5 app?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope a VCL (Borland) app. I have of course since discovered how neat pywinauto's implementation of metaclasses is such that I simply define a new class derived from this one, and override items, and that is respected and used by pywinauto (the metaclass updates a dict mapping of controls to wrappers). Which means I'm actually free to experiment willy-nilly on broader impacts. Part of me thinks the earlier solution I suggest of an optional argument click_to_find_children=False is impact neutral on existing functionality and does nothing special.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the details! I will think what we can do.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out rather that automation_id which indeed proved unreliably available, the only key I could find for a caching dict is element_info.element which seems to be a unique pointer (which Python permits as a dict key). Problem is nothing else I could find anywhere under element_info provides a unique handle on the element (include name which is blank on most of our controls).

items = self._cache_items[self.element_info.automation_id].copy()
else:
self.click_input()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

click_input() is an invasive technique which can make dump_tree() a.k.a. print_control_identifiers() blinking with the menu. Item texts are taken into account by best_match algorithm so dump_tree() can call it to obtain all possible best_match names. As I remember, I already fixed similar problem for combo box wrapper pretty recently (I mean last year :) ).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by "invasive" or what alternative there is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Invasive" means that method .dump_tree() assumed as read-only. Think about it as a C++ const method. It calls .items() for every complicated element, but your implementation of .items() changes the state of the element (not a const method in terms of C++) and it even keeps submenu open. More right way to do it is expanding the submenu explicitly in your code, call what you need (list subitems) and then collapse the menu back if you don't need it any more.

Yes, some apps have better design of menu structure so the items are invisible but accessible. In your case a dynamic action is required to list the items.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per original note above: Part of me thinks the earlier solution I suggest of an optional argument click_to_find_children=False is impact neutral on existing functionality and does nothing special. But I'm loving the easy wrapper override facility that makes experimenting with solutions easy.

if self.children():
me = self.children()[0]
items = me.children(control_type="MenuItem")
self._cache_items[self.element_info.automation_id] = items.copy()

return items

# -----------------------------------------------------------
def select(self):
Expand Down