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

[Fabric] Fix crash when running inspect #13592

Merged
merged 3 commits into from
Aug 20, 2024
Merged

Conversation

acoates-ms
Copy link
Contributor

@acoates-ms acoates-ms commented Aug 20, 2024

Description

Running inspect.exe, or likely a bunch of other accessibility tools causes RNW experiences to crash.

Why

A bad iterator over the children collection would attempt to dereference the end iterator, which throws.

Microsoft Reviewers: Open in CodeFlow

@acoates-ms acoates-ms requested a review from a team as a code owner August 20, 2024 02:51
@acoates-ms acoates-ms enabled auto-merge (squash) August 20, 2024 15:00
@acoates-ms acoates-ms merged commit a9e5ef1 into microsoft:main Aug 20, 2024
55 checks passed
Yajur-Grover pushed a commit to Yajur-Grover/react-native-windows that referenced this pull request Oct 9, 2024
* [Fabric] Fix crash when running inspect

* Change files

* fix
Yajur-Grover added a commit that referenced this pull request Oct 11, 2024
* Introduce ReactNativeAppBuilder (#13368)

* Introduce ReactNativeAppBuilder.idl

* yarn format fix

* Change files

* Address feedback

* Add a fabric check for ReactNativeAppBuilder

* yarn format

* Update reactnativeappbuilder (#13467)

* Build reactnativeappbuilder

* Setup reactnativewin32 app

* Fix dispatcherqueue issue

* Change files

* Cleanup

* Address feedback

* Address feedback2

* Fix pipeline error

* Fix pipeline errors

* Address feedback

* [Fabric] Fix crash when running inspect (#13592)

* [Fabric] Fix crash when running inspect

* Change files

* fix

* [Fabric] LogBox should destroy its window on instance shutdown (#13675)

* [Fabric] LogBox should destroy its window on instance shutdown

* Change files

* format

* [Fabric] Add FocusNavigationDirection and allow overriding of default command handling (#13857)

* [Fabric] Add FocusNavigationDirection and allow overriding of default command handling

* Change files

* revert packages.lock

* [Fabric] LayoutDirection and FontSizeMultiplier support (#13866)

* [Fabric] LayoutDirection and FontSizeMultiplier support

* Change files

* Missed a few places not setting LayoutDirection

* fix crash on logbox

* Fix textinput caret

* fix

* RichEdit has to be told its multiline before we can set a multiline string

* Update SampleAppFabric to use ReactNativeAppBuilder (#13382)

* Update SampleAppFabric to use ReactNativeAppBuilder

* Yarn format fix

* Address feedback to keep BUNDLE and DEBUG

* Yarn format

* Change files

* Setup the DebugBundlePath when loading from metro

* fix typo

* [Fabric] Enable ScrollView touch scrolling (#13664)

* [Fabric] Enable ScrolView touch scrolling

* Change files

* format

* typo

* [Fabric] Add Support for accessibilityActions and onAccessibilityAction (#13674)

* Add Implementation for accessibilityActions

* Add activate action

* Format

* Change files

* Use a legacy native module binding that always returns null in bridgeless mode (#13905)

* Use a legacy native module binding that always returns null in bridgeless mode

* Change files

* format

* SampleTurboModule only works as a turbomodule, so do not install it when using web debugger (#13911)

* SampleTurboModule only works as a turbomodule, so do not install it when using web debugger

* Change files

* fix

* Implement the SampleTurboModule module (#13541)

This PR provides a proper implementation of the `SampleTurboModule` module and removes the proxy code in `TurboModuleManager` which instead substituted the old `SampleTurboCxxModule` module.

- New feature (non-breaking change which adds functionality)

The APIs of `SampleTurboModule` are starting to deviate from the older `SampleTurboCxxModule`, specifically the addition of new `EventEmitter` members. So it's time we had a "real" implementation of `SampleTurboModule`.

Closes #13531

See above.

N/A

Verified tests still pass and the new module is being called.

Should this change be included in the release notes: _yes_

Implement the SampleTurboModule module

* fix previous merge

* fix bad merge

* fix more typos

* update snapshots

---------

Co-authored-by: Sharath Manchala <[email protected]>
Co-authored-by: Andrew Coates <[email protected]>
Co-authored-by: Chiara Mooney <[email protected]>
Co-authored-by: Jon Thysell <[email protected]>
acoates-ms added a commit to acoates-ms/react-native-windows that referenced this pull request Oct 11, 2024
* [Fabric] Fix crash when running inspect

* Change files

* fix
acoates-ms added a commit that referenced this pull request Oct 15, 2024
* Introduce ReactNativeAppBuilder (#13368)

* Introduce ReactNativeAppBuilder.idl

* yarn format fix

* Change files

* Address feedback

* Add a fabric check for ReactNativeAppBuilder

* yarn format

* Update SampleAppFabric to use ReactNativeAppBuilder (#13382)

* Update SampleAppFabric to use ReactNativeAppBuilder

* Yarn format fix

* Address feedback to keep BUNDLE and DEBUG

* Yarn format

* Change files

* Setup the DebugBundlePath when loading from metro

* Update reactnativeappbuilder (#13467)

* Build reactnativeappbuilder

* Setup reactnativewin32 app

* Fix dispatcherqueue issue

* Change files

* Cleanup

* Address feedback

* Address feedback2

* Fix pipeline error

* Fix pipeline errors

* Address feedback

* Implement the SampleTurboModule module (#13541)

This PR provides a proper implementation of the `SampleTurboModule` module and removes the proxy code in `TurboModuleManager` which instead substituted the old `SampleTurboCxxModule` module.

- New feature (non-breaking change which adds functionality)

The APIs of `SampleTurboModule` are starting to deviate from the older `SampleTurboCxxModule`, specifically the addition of new `EventEmitter` members. So it's time we had a "real" implementation of `SampleTurboModule`.

Closes #13531

See above.

N/A

Verified tests still pass and the new module is being called.

Should this change be included in the release notes: _yes_

Implement the SampleTurboModule module

* [Fabric] Fix crash when running inspect (#13592)

* [Fabric] Fix crash when running inspect

* Change files

* fix

* [Fabric] Enable ScrollView touch scrolling (#13664)

* [Fabric] Enable ScrolView touch scrolling

* Change files

* format

* typo

* [Fabric] Add Support for accessibilityActions and onAccessibilityAction (#13674)

* Add Implementation for accessibilityActions

* Add activate action

* Format

* Change files

* [Fabric] LogBox should destroy its window on instance shutdown (#13675)

* [Fabric] LogBox should destroy its window on instance shutdown

* Change files

* format

* [Fabric] Add FocusNavigationDirection and allow overriding of default command handling (#13857)

* [Fabric] Add FocusNavigationDirection and allow overriding of default command handling

* Change files

* revert packages.lock

* [Fabric] LayoutDirection and FontSizeMultiplier support (#13866)

* [Fabric] LayoutDirection and FontSizeMultiplier support

* Change files

* Missed a few places not setting LayoutDirection

* fix crash on logbox

* Fix textinput caret

* fix

* RichEdit has to be told its multiline before we can set a multiline string

* Use a legacy native module binding that always returns null in bridgeless mode (#13905)

* Use a legacy native module binding that always returns null in bridgeless mode

* Change files

* format

* SampleTurboModule only works as a turbomodule, so do not install it when using web debugger (#13911)

* SampleTurboModule only works as a turbomodule, so do not install it when using web debugger

* Change files

* fix

* merge conflicts

* format

* fix

* lint fix

* fix

* fix

* update snapshots

---------

Co-authored-by: Sharath Manchala <[email protected]>
Co-authored-by: Jon Thysell <[email protected]>
Co-authored-by: Chiara Mooney <[email protected]>
Co-authored-by: React-Native-Windows Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants