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

browser: accessibility: ensure to create only single instance #11137

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

hcvcastro
Copy link
Member

Otherwise it will create a possible pattern:

Change-Id: I54f09b50a0cef1f228c361ebefbd5b47cd35e2be
Signed-off-by: Henry Castro [email protected]

@@ -414,7 +414,7 @@ var NotebookbarAccessibility = function() {

this.initialize = function() {
setTimeout(function() {
if (window.mode.isDesktop()) {
if (window.mode.isDesktop() && !this.initialized) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about switching the mode few times?
tabbed interface -> compact interface -> tabbed interface

it will not be reinitialized again probably?
this is called in Control.UIManager.js

Copy link
Contributor

Choose a reason for hiding this comment

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

possibly we need to "reset" it on switching to classic mode / closing tabbed interface

Copy link
Member Author

Choose a reason for hiding this comment

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

what about switching the mode few times? tabbed interface -> compact interface -> tabbed interface

it will not be reinitialized again probably? this is called in Control.UIManager.js

I have no idea, how coded/design here. I just saw an inconsistency here. It looks like a life cycle issue. I supposed
it is a singleton.

Copy link
Member Author

Choose a reason for hiding this comment

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

possibly we need to "reset" it on switching to classic mode / closing tabbed interface

The user can change dynamically user interface without reload?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, view -> compact mode / tabbed mode

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, you designed the control. I just did a quick fix, no depth analysis here, I have spotted a serious issue here. It is up to you guys to decide what is best.

Copy link
Contributor

@gokaysatir gokaysatir left a comment

Choose a reason for hiding this comment

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

I think it guards against muliple initialization of the class when switches to/from notebookbar mode multiple times.

Tested, it works. Thanks :)

I guess we can get this in unless Szymon has concerns.

@gokaysatir
Copy link
Contributor

Deleting the class instance and re-creating sounds like a better solution btw. It may prevent future issues.

@hcvcastro
Copy link
Member Author

Deleting the class instance and re-creating sounds like a better solution btw. It may prevent future issues.

Yes, It is a definitive solution. Please add your patches when it is merged

@hcvcastro hcvcastro force-pushed the pr/master/A3 branch 3 times, most recently from 4f63f30 to b3fe10f Compare February 18, 2025 15:11
@vmiklos
Copy link
Contributor

vmiklos commented Feb 18, 2025

#11193 has to go in first, then please rebase. Sorry about the trouble.

Otherwise it will create a possible pattern:

<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>
<div style="height: 0px; width: 0px; overflow: hidden;"><input type="submit" role="tablist" id="accessibilityInputElement" autocomplete="off" style="height: 0px; width: 0px;"></div>

Change-Id: I54f09b50a0cef1f228c361ebefbd5b47cd35e2be
Signed-off-by: Henry Castro <[email protected]>
@hcvcastro hcvcastro merged commit b04d482 into CollaboraOnline:master Feb 19, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants