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

[share url] fixes embed links to re-add qs parameter #5900

Merged
merged 1 commit into from
Jan 13, 2016

Conversation

BigFunger
Copy link
Contributor

Fixes #5897

Re-adds the embed querystring parameter when generating embed code

return $location.absUrl();
let url = $location.absUrl();
if ($scope.shareAsEmbed) {
url = url.replace('?', '?embed=true&');
Copy link
Contributor

Choose a reason for hiding this comment

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

This really isn't how we should be handling this. This fails when there isn't a query string and when there is a query string before the hash. This should instead use require('url').parse and require('url').format.

@BigFunger
Copy link
Contributor Author

@spalger This is how it was being handled before the rework.

25c6627#diff-eaa764287bd619662df5d1a5d17486a1L241

@LeeDr
Copy link
Contributor

LeeDr commented Jan 13, 2016

LGTM I tested Jim's fix

spalger added a commit that referenced this pull request Jan 13, 2016
[share url] fixes embed links to re-add qs parameter
@spalger spalger merged commit 9661bc3 into elastic:master Jan 13, 2016
@rashidkpc
Copy link
Contributor

@BigFunger Where else was this merged? I don't see 5.0.0 or 4.5.0 on this pull? There's a user noting this still occurs: #5915

@epixa
Copy link
Contributor

epixa commented Jan 23, 2016

I don't believe this was backported, though the original issue was marked as a blocker for 4.4.0.

elasticsearch-bot pushed a commit that referenced this pull request Jan 23, 2016
@elasticsearch-bot
Copy link

courtewing merged this into the following branches!

Branch Commits
4.x 45ed4a2
4.4 aec16c0
4.4.0 6ccc9b1

elasticsearch-bot pushed a commit that referenced this pull request Jan 23, 2016
elasticsearch-bot pushed a commit that referenced this pull request Jan 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants