Prevent talkback from announcing current progress time [chromium/src : main]
Gurmeet Kalra (Gerrit)
Gurmeet Kalra added 1 comment![]( "Open in Gerrit")
Lgtm, please add a recording of the flow to the description.
Related details
- Oleh Desiatyrikov (xWF)
- Yong Li (xWF)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gurmeet Kalra (Gerrit)
Gurmeet Kalra added 2 comments![]( "Open in Gerrit")
Lgtm, please add a recording of the flow to the description.
Also, added additional comments.
boolean shouldBlock = mBlockAccessibilityEvents;The `shouldBlock` appears to be redundant. You can directly `return false` inside the if conditions where blocking is desired, and let the final `return super.requestSendAccessibilityEvent(child, event);` handle the non-blocking case.
Gurmeet Kalra (Gerrit)
Gurmeet Kalra added 1 comment![]( "Open in Gerrit")
CharSequence className = event.getClassName();
boolean isSeekBarClass =
className != null
&& (className.toString().contains("SeekBar")
|| className.toString().contains("Slider"));
boolean isTextViewClass =
(className != null && className.toString().contains("TextView"));Instead of checking class name strings, which can be fragile and very generic as part of the class name can match, could you check if `event` is an instance of the specific classes - SeekBar or Slider or TextView?
Yong Li (xWF) (Gerrit)
Yong Li (xWF) added 1 comment![]( "Open in Gerrit")
Gurmeet KalraLgtm, please add a recording of the flow to the description.
Also, added additional comments.
I personally doubt that we should do this. Because this issue happens outside of the immersive playback, and the fix will make the behavior inconsitent. If we want to apply the same approach to other seekbars, it is no longer a Chrome XR issue.
Related details
- Gurmeet Kalra
- Oleh Desiatyrikov (xWF)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
Yong Li (xWF) added 1 comment![]( "Open in Gerrit")
CharSequence className = event.getClassName();
boolean isSeekBarClass =
className != null
&& (className.toString().contains("SeekBar")
|| className.toString().contains("Slider"));
boolean isTextViewClass =
(className != null && className.toString().contains("TextView"));Instead of checking class name strings, which can be fragile and very generic as part of the class name can match, could you check if `event` is an instance of the specific classes - SeekBar or Slider or TextView?
That's the ugly part here. I have tried many other approaches, and none of them works. The problem is the element is nested in the Slide. We don't have any access.
Gurmeet Kalra (Gerrit)
Gurmeet Kalra added 1 comment![]( "Open in Gerrit")
Gurmeet KalraLgtm, please add a recording of the flow to the description.
Yong Li (xWF)Also, added additional comments.
I personally doubt that we should do this. Because this issue happens outside of the immersive playback, and the fix will make the behavior inconsitent. If we want to apply the same approach to other seekbars, it is no longer a Chrome XR issue.
Not sure, what you mean by `I personally doubt that we should do this.`.
Adding a recording to the cl description to demo the impact of the change that is it is not announcing current progress during talkback is possible, right?
Related details
- Oleh Desiatyrikov (xWF)
- Yong Li (xWF)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
Yong Li (xWF) added 1 comment![]( "Open in Gerrit")
Gurmeet KalraLgtm, please add a recording of the flow to the description.
Yong Li (xWF)Also, added additional comments.
Gurmeet KalraI personally doubt that we should do this. Because this issue happens outside of the immersive playback, and the fix will make the behavior inconsitent. If we want to apply the same approach to other seekbars, it is no longer a Chrome XR issue.
Not sure, what you mean by `I personally doubt that we should do this.`.
Adding a recording to the cl description to demo the impact of the change that is it is not announcing current progress during talkback is possible, right?
Sorry for confusing. I mean I am not sure we should try to merge this CL. I personally don't think we should merge this CL.
1. it is ugly
2. it is inconsistent with other seekbar behaviors.
Related details
- Gurmeet Kalra
- Oleh Desiatyrikov (xWF)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gurmeet Kalra (Gerrit)
Gurmeet Kalra added 1 comment![]( "Open in Gerrit")
Gurmeet KalraLgtm, please add a recording of the flow to the description.
Yong Li (xWF)Also, added additional comments.
Gurmeet KalraI personally doubt that we should do this. Because this issue happens outside of the immersive playback, and the fix will make the behavior inconsitent. If we want to apply the same approach to other seekbars, it is no longer a Chrome XR issue.
Yong Li (xWF)Not sure, what you mean by `I personally doubt that we should do this.`.
Adding a recording to the cl description to demo the impact of the change that is it is not announcing current progress during talkback is possible, right?
Sorry for confusing. I mean I am not sure we should try to merge this CL. I personally don't think we should merge this CL.
1. it is ugly
2. it is inconsistent with other seekbar behaviors.
Agree, lets discard this change and come up with better alternate to solve the problem.
Consider using `instanceof` checks or comparing with `ClassName.class.getName()` if possible for more cleaner approach.
Related details
- Oleh Desiatyrikov (xWF)
- Yong Li (xWF)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
Yong Li (xWF) added 2 comments![]( "Open in Gerrit")
Gurmeet KalraLgtm, please add a recording of the flow to the description.
Yong Li (xWF)Also, added additional comments.
Gurmeet KalraI personally doubt that we should do this. Because this issue happens outside of the immersive playback, and the fix will make the behavior inconsitent. If we want to apply the same approach to other seekbars, it is no longer a Chrome XR issue.
Yong Li (xWF)Not sure, what you mean by `I personally doubt that we should do this.`.
Adding a recording to the cl description to demo the impact of the change that is it is not announcing current progress during talkback is possible, right?
Gurmeet KalraSorry for confusing. I mean I am not sure we should try to merge this CL. I personally don't think we should merge this CL.
1. it is ugly
2. it is inconsistent with other seekbar behaviors.
Agree, lets discard this change and come up with better alternate to solve the problem.
Consider using `instanceof` checks or comparing with `ClassName.class.getName()` if possible for more cleaner approach.
Done
CharSequence className = event.getClassName();
boolean isSeekBarClass =
className != null
&& (className.toString().contains("SeekBar")
|| className.toString().contains("Slider"));
boolean isTextViewClass =
(className != null && className.toString().contains("TextView"));Yong Li (xWF)Instead of checking class name strings, which can be fragile and very generic as part of the class name can match, could you check if `event` is an instance of the specific classes - SeekBar or Slider or TextView?
That's the ugly part here. I have tried many other approaches, and none of them works. The problem is the element is nested in the Slide. We don't have any access.
- Gurmeet Kalra
- Oleh Desiatyrikov (xWF)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gurmeet Kalra (Gerrit)
Gurmeet Kalra added 1 comment![]( "Open in Gerrit")
LGTM. Also please add a recordings of the flow that prevents the talkback to announce current progress when accessibility is enabled and other one with progress on toolbar continues when no accessibility/talkback not set.
Related details
- Oleh Desiatyrikov (xWF)
- Yong Li (xWF)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |