-
Notifications
You must be signed in to change notification settings - Fork 0
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
Poll for active tab (and non-busy state) #9
Conversation
8934884
to
5f43e6b
Compare
5f43e6b
to
b5c17b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check out these recent changes, @alice ?
document_attributes = Atspi.Document.get_document_attributes(document) | ||
if "Title" in document_attributes and document_attributes["Title"]: | ||
return True | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so this is a little bit awkward, but polling only for the busy state doesn't work on Chromium family browsers. Additionally you can't just keep testing for the Title attribute, a new accessible object is created when the document is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof, I had a play with this and I see how we ended up here.
This seems about as good as we're likely to get unless we make a change to Chromium to expose the busy state.
I did notice we get a "URI" property as well as a title; I meant to comment on the Windows change that if we can check the URL rather than the title, that'll avoid some potential ambiguity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll switch the window's PR to use the url, and open a new PR to check the URL here at that time.
document_attributes = Atspi.Document.get_document_attributes(document) | ||
if "Title" in document_attributes and document_attributes["Title"]: | ||
return True | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof, I had a play with this and I see how we ended up here.
This seems about as good as we're likely to get unless we make a change to Chromium to expose the busy state.
I did notice we get a "URI" property as well as a title; I meant to comment on the Windows change that if we can check the URL rather than the title, that'll avoid some potential ambiguity.
|
||
def setup(self, product_name): | ||
def setup(self, product_name, logger): | ||
self.logger = logger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change.
|
||
logger.debug(f"Found ATSPI active tab in {iterations} iterations") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually thought we might as well leave this in here, it will only show up when debug logging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair; I guess my thinking was that it's really only interesting if you're tuning the sleep time (which was why I added it), but I can imagine it might be useful if things start being really slow or something.
Rather than waiting for the load complete event, poll for the active tab in a non-busy state or with a non-empty "title" attribute.