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

New webxdc api #4380

Merged
merged 6 commits into from
Dec 4, 2024
Merged

New webxdc api #4380

merged 6 commits into from
Dec 4, 2024

Conversation

nicodh
Copy link
Member

@nicodh nicodh commented Dec 2, 2024

resolves (partly) to #4366

Main changes:

  • openWebxdc in frontend expects a message as first argument instead a messageId
  • selfAddr in webxdcInfo holds not the users address anymore
  • WebxdcInfo message has a new optional property webxdc_href
  • click on WebxdcInfo message directly opens the map and sets a event related location.url in iFrame (optional)

@nicodh nicodh marked this pull request as draft December 2, 2024 15:19
@nicodh nicodh added the wait-for-core Waiting for an related core issue to be resolved label Dec 2, 2024
@nicodh
Copy link
Member Author

nicodh commented Dec 2, 2024

Needs next core release

@link2xt
Copy link
Collaborator

link2xt commented Dec 2, 2024

Core 1.151.3 is published

@WofWca WofWca mentioned this pull request Dec 3, 2024
- pass whole message to internalOpenWebxdc to reduce db calls
- add href to DcOpenWebxdcParameters
@nicodh nicodh marked this pull request as ready for review December 3, 2024 13:48
@nicodh nicodh requested a review from WofWca December 3, 2024 13:53
Copy link
Collaborator

@WofWca WofWca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a bunch of questions. Hate to be that guy, but this MR affects security-critical code.

Overall I think we need more comments. Clearly document function contracts and parameters, are they sanitized or not. If not, how they need to be sanitized. The assumptions, etc.

@nicodh
Copy link
Member Author

nicodh commented Dec 3, 2024

Hate to be that guy,

No worries. I appreciate critical feedback! :-)

Copy link
Collaborator

@WofWca WofWca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment about the "internetAccess" warning. Please check it out and resolve as you wish.
Otherwise it's good, though ideally I'd like more comments, and get rid of exposeInMainWorld for setLocationUrl. But this can be done later.
IMO This is ready to be merged. (but FYI a CHANGELOG entry is missing)

@nicodh nicodh merged commit 6664e64 into main Dec 4, 2024
9 of 10 checks passed
@nicodh nicodh deleted the new-webxdc-api branch December 4, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wait-for-core Waiting for an related core issue to be resolved
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants