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

Use aria-label for Preferences selector #2181

Merged
merged 4 commits into from
Nov 30, 2024

Conversation

mquevill
Copy link
Collaborator

The selector that was previously used is also used for Security Alerts (including entering in your message sync PIN) and other windows, so it would be difficult to use that style of selector. Using aria-label=Preferences seems to be the best way to address this.

The current method used elementReady() on the first element that matched, which was usually the Sync PIN window, which caused some other conflicts with how the typical code flow would work.

The extra "style" code is no longer necessary with the backend design changes. (It's probably been outdated for some time...)

Fixes #2167.

@mquevill mquevill requested a review from dusansimic June 18, 2024 01:46
@logiclrd
Copy link

logiclrd commented Jul 8, 2024

In #2167, there is a comment indicating that the reason the security dialog doesn't show up is that the root element has class hide-preferences-window. Empirically, I can confirm that removing this class makes the security dialog show up. This PR is marked as fixing #2167, but won't hide-preferences-window still be on the root element??

@mquevill
Copy link
Collaborator Author

mquevill commented Jul 8, 2024

In #2167, there is a comment indicating that the reason the security dialog doesn't show up is that the root element has class hide-preferences-window. Empirically, I can confirm that removing this class makes the security dialog show up. This PR is marked as fixing #2167, but won't hide-preferences-window still be on the root element??

The hide-preferences-window class affects all pop-ups (preferences window, sync messages window, etc.) and should be removed after the hidden preferences window is closed. The issue has been that the function isPreferencesOpen() would return true even if the "Sync now" dialog was open. By switching to the aria-label, it's more specific to the preferences window only. This means that the code will correctly detect when that window closes and the hide-preferences-window class can be removed.

@mquevill mquevill force-pushed the UpdatePreferencesSelector branch from 66bacbd to d804634 Compare November 29, 2024 22:57
Now with updated main from upstream
@semikolon
Copy link

I just upgraded to 2.60.2 and it does not successfully remove the "hide-preferences-window" class on the html tag. Don't know why. Messenger is set to English if it matters.

@vibl
Copy link

vibl commented Dec 2, 2024

I just upgraded to 2.60.2 and it does not successfully remove the "hide-preferences-window" class on the html tag. Don't know why. Messenger is set to English if it matters.

Same for 2.60.3 here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Messages are missing. Sync now." does nothing
4 participants