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

Fix opening links in desktop IDE #6507

Merged
merged 3 commits into from
May 5, 2023
Merged

Conversation

somebody1234
Copy link
Contributor

@somebody1234 somebody1234 commented May 2, 2023

Pull Request Description

Fixes #6438
Fixes cloud-v2/#413

Screencasts

(an error dialog no longer appears)

screen-recorder-tue-may-02-2023-23-25-22.mp4

Important Notes

None

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@somebody1234
Copy link
Contributor Author

adding @wdanilo as a reviewer as he is familiar with lib/client//electron in general

@somebody1234
Copy link
Contributor Author

somebody1234 commented May 2, 2023

this may not be a particularly clean solution, however i can't think of a better one - suggestions are more than welcome though

@somebody1234 somebody1234 added the CI: No changelog needed Do not require a changelog entry for this PR. label May 2, 2023
@somebody1234 somebody1234 mentioned this pull request May 2, 2023
5 tasks
@wdanilo
Copy link
Member

wdanilo commented May 3, 2023

I believe this is the correct solution here :)

@somebody1234
Copy link
Contributor Author

@PabloBuchu @indiv0 could one of you possibly QA this?
@vitvakatu you opened #6438 so you may want to confirm this fixes the issue

things to check:

  • -startup.entry=_ then clicking on the link
  • clicking the help button on dashboard. both of these should work (and should still be failing on develop)
  • trying to break this, if possible. is it possible to navigate the electron windows away to another origin(/domain)? (not desired since this is a security feature, after all)

@PabloBuchu
Copy link
Contributor

PabloBuchu commented May 4, 2023

@somebody1234 QA 🟡

  • entry points mostly works (profile panics with missing required parameter error and profiling_run_graph complains on missing /assets/profile.json)

| clicking the help button on dashboard. both of these should work (and should still be failing on develop)
not sure I understand what exactly should work here? Currently its not failing but it also do nothing. I can see in the console Prevented navigation to 'https://discord.gg/enso'.

| trying to break this, if possible. is it possible to navigate the electron windows away to another origin(/domain)? (not desired since this is a security feature, after all)

I can type window.location.href="https://github.com" in the console and it gets me Github webpage

@somebody1234
Copy link
Contributor Author

somebody1234 commented May 4, 2023

some entry points panic

might want to check that this PR is the cause - if not then maybe create an issue for it, but i'll try to repro real quick anyway

(update: can repro but not sure this PR is the cause. i think i should do other stuff first, maybe revisit this if i have time)

Prevented navigation to discord

whoops, forgot to whitelist discord

window.location.href="https://github.com"

this works because github is whitelisted, and is (apparently) required for authentication to work

@PabloBuchu
Copy link
Contributor

Sorry I wasn't clear the non working debug scenes are not related to this PR.. I just mentioned that they are not working.

Looks like everything works so QA 🟢

@somebody1234 somebody1234 added the CI: Keep up to date Automatically update this PR to the latest develop. label May 5, 2023
@somebody1234
Copy link
Contributor Author

some gui cis are broken, merging with develop has been working for some other PRs so i figured i'd try here

@somebody1234 somebody1234 removed the CI: Keep up to date Automatically update this PR to the latest develop. label May 5, 2023
@vitvakatu
Copy link
Contributor

It fixes the issue I had with opening demo scenes. I didn't check anything besides that

@PabloBuchu PabloBuchu merged commit e1b4019 into develop May 5, 2023
@PabloBuchu PabloBuchu deleted the wip/sb/fix-opening-links branch May 5, 2023 13:49
Procrat added a commit that referenced this pull request May 10, 2023
* develop: (28 commits)
  Add tests for Date.until, Date.next and Date.previous. (#6606)
  Improve `Non_Unique_Primary_Key` error, split file format detection into read/write, improve SQLite format detection (#6604)
  tokenize_to_columns or parse_to_columns results in a single column we shouldn't add the  1 (#6607)
  Fix node editing race condition (#6594)
  Add format to the in-memory Column (#6538)
  Fix dashboard issues (part 2) (#6511)
  Fix visualisation type selector artifacts rendered after node preview visualisation was closed. (#6575)
  Revert typescript CI Lint changes (#6602)
  Fix the Engine version check in GUI (#6570)
  Show error pop-up when failing to rename a project (#6366)
  Small changes from Book Club issues (#6533)
  "at_least_one" flag for tokenize_to_rows (#6539)
  Benchmark Engine job runs only engine, not Enso benchmarks (#6534)
  Catch 5813 and avoid crash (#6585)
  Fix opening links in desktop IDE (#6507)
  Identify SyntaxError exception and avoid printing a stack trace (#6574)
  Fix dashboard issues (#6502)
  Let ChangesetBuilder.invalidated search even container elements (#6548)
  Fix #5075: stop panning on full-screen visualisation (#6530)
  Only `Join_Kind.Inner` removes the common-named columns (#6564)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't open custom entry point from the list of available ones
4 participants