Skip to content

maybe fix auth in iframes#1261

Merged
pngwn merged 8 commits into
mainfrom
login
May 16, 2022
Merged

maybe fix auth in iframes#1261
pngwn merged 8 commits into
mainfrom
login

Conversation

@pngwn
Copy link
Copy Markdown
Member

@pngwn pngwn commented May 15, 2022

  • close the loader when the login form loads
  • use a different mechanism to open link in a new tab that only affects links (a tags) rather than form actions as well.

@pngwn pngwn requested a review from abidlabs May 15, 2022 00:52
@pngwn
Copy link
Copy Markdown
Member Author

pngwn commented May 15, 2022

Difficult to test since the main issue is only present in spaces/ iframes.

@abidlabs
Copy link
Copy Markdown
Member

Actually @aliabid94 would you be able to review this?

@aliabid94
Copy link
Copy Markdown
Contributor

ok will test

Comment thread gradio/templates/frontend/index.html Outdated

async function handle_mount() {
await tick();

Copy link
Copy Markdown
Contributor

@aliabid94 aliabid94 May 15, 2022

Choose a reason for hiding this comment

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

why are we affecting all links on the page?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because we want to open them all in a new tab (including any links that are generated via markdown, which is why we need to do it this way), this was previously being done with the base element but that also affects form actions which we don't want because of the login form in iframes.

@aliabid94
Copy link
Copy Markdown
Contributor

I didn't quite understand how this fixed the problem, but auth works locally when I tested it at least. LGTM otherwise

@pngwn
Copy link
Copy Markdown
Member Author

pngwn commented May 15, 2022

Yeah sorry, I didn't elaborate.

A while ago we added the base element with a target="_blank" to open all links in a new tab. Unfortiunately this also affects form actions. I think that is what is break auth in iframes because, opening the form action link in a new tab causes it to hang and fail. We want the form action to open the url in the same window (the iframe). I don't know for certain that it will fix it but I think it will

Comment thread gradio/templates/frontend/index.html Outdated
Copy link
Copy Markdown
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

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

Unfortunately, does not seem to work on Hugging Face Spaces.

I released gradio==3.0b8 with the changes in this branch, and it works locally and with share=True. However, it doesn't seem to work on Spaces. Here's a sample Space I bult to test: https://huggingface.co/spaces/abidlabs/hello-login

@abidlabs abidlabs dismissed their stale review May 16, 2022 10:05

Postpone fix

@pngwn pngwn merged commit b69e8cb into main May 16, 2022
@pngwn pngwn deleted the login branch May 16, 2022 10:07
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.

3 participants