cobalt
cobalt copied to clipboard
Move Accessibility API from Starboard to Extension
b/299639708
Change-Id: Ida75ca986b88ae444684321ae609e1aae5588bf5
Codecov Report
Attention: Patch coverage is 1.47059%
with 67 lines
in your changes missing coverage. Please review.
Project coverage is 58.83%. Comparing base (
84bd543
) to head (fae7e35
). Report is 8 commits behind head on main.
:exclamation: Current head fae7e35 differs from pull request most recent head edd03eb
Please upload reports for the commit edd03eb to get more accurate results.
Files | Patch % | Lines |
---|---|---|
cobalt/dom/captions/system_caption_settings.cc | 0.00% | 43 Missing :warning: |
cobalt/h5vcc/h5vcc_accessibility.cc | 0.00% | 24 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #2426 +/- ##
==========================================
+ Coverage 58.74% 58.83% +0.09%
==========================================
Files 1781 1905 +124
Lines 85283 93350 +8067
==========================================
+ Hits 50099 54923 +4824
- Misses 35184 38427 +3243
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@hlwarriner could you please help review this ? I think we may need some bracketing of #ifdef STARBOARD 16 in the old headers, on an off chance someone somewhere included them.
@hlwarriner could you please help review this ? I think we may need some bracketing of #ifdef STARBOARD 16 in the old headers, on an off chance someone somewhere included them.
Sure. Yeah I think this needs SB_API_VERSION
guards, similar to how they were used in https://github.com/youtube/cobalt/pull/1930.
@hlwarriner could you please help review this ? I think we may need some bracketing of #ifdef STARBOARD 16 in the old headers, on an off chance someone somewhere included them.
Sure. Yeah I think this needs
SB_API_VERSION
guards, similar to how they were used in #1930.
I have added SB_API_VERSION guards.
@kaidokert, should someone with domain knowledge about the accessibility API take a quick look, too? I only focused on the mechanics of changing the Starboard API to an extension.
@johnxwork, can you have a quick look ?
Further - Android code still references starboard/accessibility.h
and accessibility_internal.h
here - this client code needs to be converted too, otherwise we can't land this change.
Internal CL 285260 at go/cobalt-cl/285260
I've added the Android conversion. Tests are still to be followed up on, but otherwise ready.
Internal CL needs to be merged together with this
I am working on unit test and have some issue with debugging them. When I fix it I will update this PR to add the unit tests. Also I have a question. There is a comment, as I remember, in the CL to rename StarboardExtensionAccessibilityApi to CobaltExtensionAccessibilityApi but we steel have StarboardExtensionAccessibilityApi here and CobaltExtensionAccessibilityApi in the CL. I missed these changes I suppose when I was resolving conflicts. Which name should I used? I will update in the next commit.
I am working on unit test and have some issue with debugging them. When I fix it I will update this PR to add the unit tests.
Lets add those in a separate PR as a follow-up please. We are getting a bit time critical to get the Starboard header changes in, so i'm okay to do those later.
Also I have a question. There is a comment, as I remember, in the CL to rename StarboardExtensionAccessibilityApi to CobaltExtensionAccessibilityApi but we steel have StarboardExtensionAccessibilityApi here and CobaltExtensionAccessibilityApi in the CL. I missed these changes I suppose when I was resolving conflicts. Which name should I used? I will update in the next commit.
The naming should generally be "Starboard Extension" everywhere. Again this is okay to follow-up with cleanup CLs.
Most important here is that i can merge this with green builds together with internal CLs 285260 and 285223