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

MSC1716: Open on device API #1716

Open
wants to merge 5 commits into
base: old_master
Choose a base branch
from

Conversation

Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Nov 12, 2018

@Half-Shot Half-Shot changed the title Placeholder for issue # MSC1716 Open on device API Nov 12, 2018
@Half-Shot Half-Shot changed the title MSC1716 Open on device API MSC1716: Open on device API Nov 12, 2018
@turt2live turt2live added proposal A matrix spec change proposal proposal-pr T-Core labels Nov 12, 2018

The recipient device ID should be the stored preference stated previously.

When the message arrives on the recipient device, the device must immediately change the current view to show the type in the contents. It is not
Copy link
Member

@uhoreg uhoreg Nov 12, 2018

Choose a reason for hiding this comment

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

I would suggest that clients should prompt the user rather than immediately changing display. This would help mitigate the two listed potential issues.


```json
{
"type": "room",
Copy link
Member

Choose a reason for hiding this comment

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

Instead of specifying different types, you could also use matrix:// URLs (when that lands), and just have a single href property.


### Schema

The event type should be `m.openondevice` and the `EventContent` should be:
Copy link
Member

Choose a reason for hiding this comment

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

bikeshedding: m.open_on_device instead of m.openondevice. Though the "on device" is somewhat redundant since it's a to_device message. So maybe just m.open, or m.goto to match the /goto command

| type | string | One of "room", "event", "user" or "group" | Non-optional |
| room_id | string | A room ID when the type is "event" | |
| via [1] | string[] | A set of servers needed for "room" and "event" | [] |
| id | string | A room ID, event ID, user ID | Non-optional |
Copy link
Member

Choose a reason for hiding this comment

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

Even if we're not soon getting nice matrix:// URIs mentioned above, I'm not quite sure what you get by producing a separate type field. Why not just use room (standing for id or alias - they can be easily discerned by a sigil), user, event, and group as keys? E.g.,

"content": { "room": "!aroomfor:allof.us" }

would be used to open a room (modulo via), while

"content": {
    "room": "#allofus:allof.us",
    "event": "$event:allof.us"
}

would be the same for events.

That wouldn't work if you want to make this system client-extensible (with, say, Riot using its own "type": "im.riot.popup" standing for a popup specifically within Riot clients) but I seriously doubt in such extensibility being worthwhile or even desired.


| key | type | value | Default |
|---------|----------|------------------------------------------------|--------------|
| type | string | One of "room", "event", "user" or "group" | Non-optional |
Copy link
Member

Choose a reason for hiding this comment

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

Non-optional -> required, or mandatory?

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

in general, why not an m.open event like this:

{
  "uri": "https://matrix.to/#/!somewhere:example.org"
}

Assuming the documentation reads similar to "the uri is the location to open, which may current be a [matrix.to] URI or in the future some sort of matrix://-scheme resource".

```json
{
"type": "room",
"via": "half-shot.uk",
Copy link
Member

Choose a reason for hiding this comment

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

this should be an array

{
"type": "event",
"room_id": "!someneatlookingroom:matrix.org",
"via": "half-shot.uk",
Copy link
Member

Choose a reason for hiding this comment

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

again: this should be an array

{
"type": "room",
"via": "half-shot.uk",
"id": "!someneatlookingroom:matrix.org",
Copy link
Member

Choose a reason for hiding this comment

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

what if the room is an alias?

}
```

| key | type | value | Default |
Copy link
Member

Choose a reason for hiding this comment

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

formatting: this table is under group, not a more general location.

| key | type | value | Default |
|---------|----------|------------------------------------------------|--------------|
| type | string | One of "room", "event", "user" or "group" | Non-optional |
| room_id | string | A room ID when the type is "event" | |
Copy link
Member

Choose a reason for hiding this comment

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

surely this is required under some circumstances?

|---------|----------|------------------------------------------------|--------------|
| type | string | One of "room", "event", "user" or "group" | Non-optional |
| room_id | string | A room ID when the type is "event" | |
| via [1] | string[] | A set of servers needed for "room" and "event" | [] |
Copy link
Member

Choose a reason for hiding this comment

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

... it's an array down here but not above :(

(see previous comments)

@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff and removed proposal-pr labels Apr 20, 2020
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants