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

electron: Update to 27.0.3. #3927

Closed
wants to merge 1 commit into from

Conversation

JoeGruffins
Copy link
Member

closes #3921

@matheusd
Copy link
Member

matheusd commented Nov 7, 2023

Tested on windows, linux and macOS.

On macOS it worked fine.

On Windows the rebuild-natives call failed due to node-abi not supporting electron 27.0.3. I had to downgrade to 26.4.3 to rebuild native modules. DEX worked on that version.

On Linux, DEX did not open, and downgrading to 26.4.3 also did not make it work (neither in testnet nor mainnet). Still trying to work out why or which version is able to run it.

@alexlyp
Copy link
Member

alexlyp commented Nov 7, 2023

@matheusd That was what I encountered as well. I was able to narrow it down to something in https://github.com/electron/electron/releases/tag/v21.3.0 that was causing it to not work on Linux anymore. Hence why I rolled back to 21.2.3. Afaict that's the last working version of electron. And the only change in there that could possibly have a difference is a Chromium update.

@matheusd
Copy link
Member

matheusd commented Nov 7, 2023

While debugging I set the dex window to openDevTools and I get an error there. That may or may not be the issue. I'm retesting on master and then will test on 21.3.0 to see if that's the issue

@matheusd
Copy link
Member

matheusd commented Nov 7, 2023

This is the error I get starting on electron 21.3.0 when opening the dex window (this is the console for the dex window, not the main decrediton window):

image

@matheusd
Copy link
Member

matheusd commented Nov 7, 2023

I expect something in chromium broke a lib that the dex interface uses on linux.

@matheusd
Copy link
Member

matheusd commented Nov 7, 2023

I believe this is the issue:

navigator.languages

  • on 21.2.3: ['en-US']
  • on 21.3.0: ['en-US', 'c']

More precisely, passing 'c' as one of the options for Intl.NumberFormat() causes the issue, so this needs to be filtered.

We'll probably need to check with the DEX UI ppl how to fix this

@JoeGruffins
Copy link
Member Author

On Windows the rebuild-natives call failed due to node-abi not supporting electron 27.0.3. I had to downgrade to 26.4.3 to rebuild native modules. DEX worked on that version.

@matheusd Shall I make this upgrade to 26.4.3? That also solves the main ledger issue.

@matheusd
Copy link
Member

matheusd commented Nov 8, 2023

@matheusd Shall I make this upgrade to 26.4.3? That also solves the main ledger issue.

Yes please. This should fix the windows build, though there's still the issue of the DEX UI failing on linux.

@alexlyp
Copy link
Member

alexlyp commented Nov 8, 2023

Closing this one in favor of another that includes the dcrdex fix as well.

@alexlyp alexlyp closed this Nov 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.

Ledger: Broken on master.
3 participants