Skip to content

Fix issue 8012 and 7844 - #8037

Closed
ayushmanchhabra wants to merge 25 commits into
nwjs:nw75from
ayushmanchhabra:dev-8012
Closed

Fix issue 8012 and 7844#8037
ayushmanchhabra wants to merge 25 commits into
nwjs:nw75from
ayushmanchhabra:dev-8012

Conversation

@ayushmanchhabra

@ayushmanchhabra ayushmanchhabra commented Mar 3, 2023

Copy link
Copy Markdown
Contributor

Fixes: #8012
Fixes: #7844

The Window.showDevtools function crashes since it is not available on normal flavor. This PR makes the function throw an error which the user can catch in a try/catch block preventing crash.

@ayushmanchhabra ayushmanchhabra changed the title Prevent crash on nw.showDevTools on non-sdk build feat: exit nw.showDevTools function if called on non-sdk build Mar 3, 2023
@ayushmanchhabra ayushmanchhabra self-assigned this Mar 7, 2023
@ayushmanchhabra
ayushmanchhabra marked this pull request as ready for review March 7, 2023 08:33
@ayushmanchhabra

Copy link
Copy Markdown
Contributor Author

I was not sure how to test for this behaviour - please let me know if there is a better way to go about it!

Comment thread .gitignore Outdated
Comment thread test/sanity/window-showdevtools/index.html Outdated
Comment thread test/sanity/window-showdevtools/package.json Outdated
Comment thread test/sanity/window-showdevtools/index.html Outdated
TheJaredWilcurt and others added 4 commits March 8, 2023 00:39
Sanity tests are executed by chromedriver on sdk build flavor. We want to test if showDevTools does not crash NW on execution.
@ayushmanchhabra ayushmanchhabra changed the title feat: exit nw.showDevTools function if called on non-sdk build feat: exit showDevTools if called on non-sdk build Mar 9, 2023
@ayushmanchhabra
ayushmanchhabra marked this pull request as draft March 10, 2023 02:29
@ayushmanchhabra
ayushmanchhabra changed the base branch from nw73 to nw74 March 10, 2023 02:30
@ayushmanchhabra

ayushmanchhabra commented Mar 15, 2023

Copy link
Copy Markdown
Contributor Author

@rogerwang This is ready for review.

Would you be open to backporting these changes to previous versions (nw72 for win 7 and 8)?

@ayushmanchhabra
ayushmanchhabra marked this pull request as ready for review March 15, 2023 05:59
@ayushmanchhabra

Copy link
Copy Markdown
Contributor Author

@rogerwang would like to get some feedback on this whenever you get the time

@ayushmanchhabra ayushmanchhabra mentioned this pull request Apr 3, 2023
@ayushmanchhabra
ayushmanchhabra marked this pull request as draft April 4, 2023 20:59
@ayushmanchhabra
ayushmanchhabra marked this pull request as ready for review April 5, 2023 04:46
@ayushmanchhabra ayushmanchhabra changed the title feat: exit showDevTools if called on non-sdk build Fix issue 8012 and 7844 Apr 17, 2023
@ayushmanchhabra
ayushmanchhabra marked this pull request as draft April 18, 2023 00:56
@ayushmanchhabra
ayushmanchhabra changed the base branch from nw74 to nw75 April 20, 2023 19:23
@ayushmanchhabra
ayushmanchhabra marked this pull request as ready for review April 20, 2023 19:30
@ayushmanchhabra

ayushmanchhabra commented Apr 20, 2023

Copy link
Copy Markdown
Contributor Author

@TheJaredWilcurt feel free to update/improve the JSDoc descriptions. I was not sure how to document a callback - maybe you can help out with that?

Comment thread src/resources/api_nw_newwin.js
Comment thread src/resources/api_nw_window.js
Comment thread docs/References/Window.md Outdated
Comment thread src/resources/api_nw_newwin.js
Co-authored-by: The Jared Wilcurt <TheJaredWilcurt@users.noreply.github.com>
@ayushmanchhabra ayushmanchhabra closed this by deleting the head repository May 8, 2023
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.

Non-SDK nw.js fails on showDevTools DevTools segafult

2 participants