Stop Discover's Visualize button from loading incorrect URL#8721
Merged
Bargs merged 1 commit intoelastic:masterfrom Oct 17, 2016
Merged
Stop Discover's Visualize button from loading incorrect URL#8721Bargs merged 1 commit intoelastic:masterfrom
Bargs merged 1 commit intoelastic:masterfrom
Conversation
When App state is read from the url, it is accessed via `$location.search()`. It turns out that the result of `$location.search()` is a mutable object that allows you to modify the results of `$location.search()` without actually modifying the URL. When a user clicks a field in the Discover sidebar, it executes the scope method `vizLocation` to get the href value for the "Visualize" button. As an unintended side effect, `vizLocation` was modifying the object returned from `$location.search()`. This change would ultimately be read when the Visualize app loaded instead of the param in the actual URL since there's no full page load from Discover -> Visualize. To make things more complicated, I believe there was a race condition partially masking this issue. Since `vizLocation` is used in an Angular string template, the data binding causes it to execute on every digest cycle. This is why the incorrect value continued to be written to `$location.search()` even after `visLocation` ran for the correct field. It also meant that the result of `$location.search()` was totally dependent on which field `vizLocation` ran for most recently before the user clicked Visualize. Fixes elastic#8718
Contributor
Author
|
I just confirmed this issue existed in 4.x as well so I think the fix can wait until 5.1. |
jbudz
approved these changes
Oct 17, 2016
thomasneirynck
approved these changes
Oct 17, 2016
elastic-jasper
added a commit
that referenced
this pull request
Oct 17, 2016
--------- **Commit 1:** Stop Discover's Visualize button from loading incorrect URL When App state is read from the url, it is accessed via `$location.search()`. It turns out that the result of `$location.search()` is a mutable object that allows you to modify the results of `$location.search()` without actually modifying the URL. When a user clicks a field in the Discover sidebar, it executes the scope method `vizLocation` to get the href value for the "Visualize" button. As an unintended side effect, `vizLocation` was modifying the object returned from `$location.search()`. This change would ultimately be read when the Visualize app loaded instead of the param in the actual URL since there's no full page load from Discover -> Visualize. To make things more complicated, I believe there was a race condition partially masking this issue. Since `vizLocation` is used in an Angular string template, the data binding causes it to execute on every digest cycle. This is why the incorrect value continued to be written to `$location.search()` even after `visLocation` ran for the correct field. It also meant that the result of `$location.search()` was totally dependent on which field `vizLocation` ran for most recently before the user clicked Visualize. Fixes #8718 * Original sha: d0d926b * Authored by Matthew Bargar <mbargar@gmail.com> on 2016-10-17T21:07:11Z
airow
pushed a commit
to airow/kibana
that referenced
this pull request
Feb 16, 2017
--------- **Commit 1:** Stop Discover's Visualize button from loading incorrect URL When App state is read from the url, it is accessed via `$location.search()`. It turns out that the result of `$location.search()` is a mutable object that allows you to modify the results of `$location.search()` without actually modifying the URL. When a user clicks a field in the Discover sidebar, it executes the scope method `vizLocation` to get the href value for the "Visualize" button. As an unintended side effect, `vizLocation` was modifying the object returned from `$location.search()`. This change would ultimately be read when the Visualize app loaded instead of the param in the actual URL since there's no full page load from Discover -> Visualize. To make things more complicated, I believe there was a race condition partially masking this issue. Since `vizLocation` is used in an Angular string template, the data binding causes it to execute on every digest cycle. This is why the incorrect value continued to be written to `$location.search()` even after `visLocation` ran for the correct field. It also meant that the result of `$location.search()` was totally dependent on which field `vizLocation` ran for most recently before the user clicked Visualize. Fixes elastic#8718 * Original sha: a839d2bbfef59996f51f2e2ddacf2e5d1a1d9fd3 [formerly d0d926b] * Authored by Matthew Bargar <mbargar@gmail.com> on 2016-10-17T21:07:11Z Former-commit-id: e7e228f
airow
pushed a commit
to airow/kibana
that referenced
this pull request
Feb 16, 2017
[backport] PR elastic#8721 to 5.x Former-commit-id: 39a4f3b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When App state is read from the url, it is accessed via
$location.search(). It turns out that the result of$location.search()is a mutable object that allows you to modify theresults of
$location.search()without actually modifying the URL. Whena user clicks a field in the Discover sidebar, it executes the scope
method
vizLocationto get the href value for the "Visualize" button.As an unintended side effect,
vizLocationwas modifying the objectreturned from
$location.search(). This change would ultimately be readwhen the Visualize app loaded instead of the param in the actual URL
since there's no full page load from Discover -> Visualize.
To make things more complicated, I believe there was a race condition
partially masking this issue. Since
vizLocationis used in an Angularstring template, the data binding causes it to execute on every digest
cycle. This is why the incorrect value continued to be written to
$location.search()even aftervisLocationran for the correct field.It also meant that the result of
$location.search()was totallydependent on which field
vizLocationran for most recently before theuser clicked Visualize.
Fixes #8718