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

What should happen when ctrl+clicking on an intra-grain link? #1848

Open
mitar opened this issue Apr 15, 2016 · 16 comments
Open

What should happen when ctrl+clicking on an intra-grain link? #1848

mitar opened this issue Apr 15, 2016 · 16 comments

Comments

@mitar
Copy link
Contributor

mitar commented Apr 15, 2016

I see in the Rocket.Chat app that I can open internal frame directly in the browser. Shouldn't the app redirect to Sandstorm URL in this case? So always require the frame to be around? Is this problem of Rocket.Chat?

@kentonv
Copy link
Member

kentonv commented Apr 15, 2016

We don't consider this a problem. Opening the grain host in a window without the Sandstorm chrome doesn't allow the app to do anything bad. Mostly it hurts the app because if the grain tab in the sandstorm UI is closed then the app server will shut down and any external windows will stop working.

@kentonv kentonv closed this as completed Apr 15, 2016
@mitar
Copy link
Contributor Author

mitar commented Apr 18, 2016

@kentonv: Hm, it seems there is no way really for grain to know what is the URL of the grain in Sandstorm tab? So there is no way to redirect back to Sandstorm?

For me this is a question of usability. Users might open a link by mistake in a new tab. It should get Sandstorm UI automatically.

@kpreid
Copy link

kpreid commented Apr 18, 2016

By mistake? How about users opening links on purpose in a new tab? That not working in the straightforward way (Sandstorm UI always present) is a problem. Can we reopen this, please?

@elimisteve
Copy link
Contributor

@mitar Why do you need the full URL? Can't use relative URLs and just specify the path, (e.g., /app/users/whatever)?

@mitar
Copy link
Contributor Author

mitar commented Apr 20, 2016

Because sandstrom sessions run in their own origin. You have to know how to redirect back to Sandstorm with full URL.

@mitar
Copy link
Contributor Author

mitar commented Apr 22, 2016

Hm, it seems I can get full URL from window.parent.location. So I should just associate this from the client for a particular iframe origin. And then a page is opened with window.parent for the same iframe origin, I redirect to window.parent.location.

But it would be much better if there would be a HTTP header for this send to the app.

@paulproteus
Copy link
Collaborator

paulproteus commented Apr 22, 2016 via email

@mitar
Copy link
Contributor Author

mitar commented Apr 22, 2016

BTW, redirect should be done to the path of an grain as well, not just to the top URL of the grain.

@zarvox
Copy link
Collaborator

zarvox commented Apr 22, 2016

@mitar I don't think you can actually get the full URL from window.parent.location - are you running that in the context of the grain iframe? It should be sandboxed.

I believe @kentonv had defensible privacy/anti-worm reasons for not wanting grains to be able to obtain references to themselves without explicit user interaction. I'm not sure if the .well-known/sandstorm/grain-redirect proposal preserves these security properties, but if it does, then I'm +1 on that idea. I think it does if we enable it only for web-session (but not api-session) because it can't be used outside the context of the current user's session.

@mitar
Copy link
Contributor Author

mitar commented Apr 22, 2016

@mitar I don't think you can actually get the full URL from window.parent.location - are you running that in the context of the grain iframe? It should be sandboxed.

I tried on Oasis in Firefox in one grain through web console, switched to iframe context and wrote window.parent.location and it prints it out. But you are right. I can access the object, but I cannot really do anything with it. So the console can print it. But if I do "" + window.parent.location to convert it to string, it fails.

@mitar
Copy link
Contributor Author

mitar commented Apr 22, 2016

Can then this ticket be reopened and let's do this redirect? That would be great to have. In some way it would be inverse of my code here. Instead of posting to parent the current location, the app would redirect to /.well-known/sandstorm/grain-redirect/<path> with current path.

@kentonv kentonv reopened this Apr 22, 2016
@mitar mitar changed the title Who's responsibility is to redirect directly opened URLs to the Sandstorm URL? Supporting apps redirecting directly opened URLs to the Sandstorm URL Apr 22, 2016
@kentonv
Copy link
Member

kentonv commented Apr 22, 2016

This proposal still seems problematic to me. The popup window will load a whole new copy of Sandstorm with tabs and everything -- is that really what you want?

Maybe what we should really have here is a new postMessage API for "Please open a copy of this grain in a popup window"? That way the popup can share a Javascript context and can perhaps be rendered in a lightweight version of the shell.

Though there's a problem that the browser's popup blocker will block popups created in response to a postMessage, since it will lose association with the user click.

I wonder why you want a popup window at all. Popups are pretty awful UX most of the time. I am almost inclined to ban apps from making popups (which we could enforce with iframe sandboxing), except that it would break HackerSlides' presentation mode...

@mitar
Copy link
Contributor Author

mitar commented Apr 22, 2016

I was not thinking about popups. I am thinking about a simple grain like: https://alpha.sandstorm.io/grain/kB7bofATTL2jGuKSFwmqdA/GithubPRCommands

Click with command-click or whatever you have in your browser to open a link in a tab, on any link in the sidebar of MediaWiki. It opens the page in new tab. Now, that tab is not in Sandstorm anymore. It should be. The question is how to get it to be in Sandstorm.

Redirect is one option. But it would be great if it could be the same session (so same origin). Not another Sandstorm instance.

@kentonv kentonv changed the title Supporting apps redirecting directly opened URLs to the Sandstorm URL What should happen when ctrl+clicking on an intra-grain link? Apr 22, 2016
@kentonv
Copy link
Member

kentonv commented Apr 22, 2016

OK, I changed the bug title to reflect this.

You could argue that ctrl+click on an intra-grain link should open a sidebar tab, although we currently expect to have only one sidebar tab for each grain.

This question might also relate to a separate feature request (not sure if there's an open issue): Being able to "pop out" a grain into a separate window.

@mitar
Copy link
Contributor Author

mitar commented Apr 22, 2016

You could argue that ctrl+click on an intra-grain link should open a sidebar tab

This interferes with user's browsing habits and user's use of my browser UI, screen-estate and so on. On the mobile phone user might want a link to open in a new background window, not in a in-window sidebar.

But it is one option. The best would be if we could support native browsing somehow.

@kpreid
Copy link

kpreid commented Apr 23, 2016

It is, in my opinion, very important that Sandstorm not harm the things that make web apps web apps rather than just apps. If open-in-new-tab is either nonstandard (Sandstorm sidebar) or deficient (missing UI) then this is bad.

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

No branches or pull requests

6 participants