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

[WIP] Add storage interactions in the bottom-drawer #397

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Pamplemousse
Copy link
Contributor

@Pamplemousse Pamplemousse commented Feb 6, 2019

Using the front-end-tracker to get storage interactions, then interface that to the HUD's bottom-drawer.

Here a couple of things to have a look at before integrating to the HUD:

  • Do not freeze the HUD when the storage is flooded (for example in juiceshop, where a constant stream of getItem is happening)
  • Make the UI more consistent with the history tab
  • (optional) Add missing features: modal for details of an entry, filtering and regex.
  • Opt-in / opt-out per site

@psiinon
Copy link
Member

psiinon commented Feb 22, 2019

@Pamplemousse have you got any sites you recommend testing with?
I tried https://www.w3schools.com/html/tryit.asp?filename=tryhtml5_webstorage_local but nothing appeared when I ran the example :/

@psiinon
Copy link
Member

psiinon commented Feb 22, 2019

https://jsfiddle.net/Benjol/HMEVd/ failed too :/

@Pamplemousse Pamplemousse force-pushed the develop branch 2 times, most recently from 959ea83 to d747ecd Compare March 5, 2019 13:22
@psiinon
Copy link
Member

psiinon commented Mar 5, 2019

This pull request introduces 1 alert when merging d747ecd into 1fdbb47 - view on LGTM.com

new alerts:

  • 1 for Syntax error

Comment posted by LGTM.com

@Pamplemousse
Copy link
Contributor Author

@psiinon In both examples, the storage tab in the bottom-drawer is working, as long as the code interacting with the Storage API is run in the domain on which the HUD is effectively run.


On https://www.w3schools.com/html/tryit.asp?filename=tryhtml5_webstorage_local

The demo code seems to be written in an iframe.
This defeats the front-end-tracker as the Storage interface's functions inside the iframe cannot be instrumented, so the "reporting code" is not injected.
If, from the console, you try localStorage.removeItem('lastname'), then the event would show up in the bottom-drawer.

On https://jsfiddle.net/Benjol/HMEVd/

Investigating with the developer console, it appears that the localStorage concerned by the example is fiddle.jshell.net, and not jsfiddle.net.
Reading the source of the page, again, the example is run inside an iframe, and indeed, it sources a document hosted on another domain.
To see the tab work: try resizing the grid in the page, and watch for the grid_1-split-sizes value to be written in the localStorage, and correctly reported in the bottom-drawer.


Just as a mention (not to be tackled)
As the front-end-tracker has been designed, it cannot report code execution that has not been instrumented, and it does that when injected on top of a page.
Covering the cases of instrumenting code in an iframe would need some serious thinking, and surely some new design...

Will be needed to feed the bottom-drawer with storage or DOM events.
There will be other tabs, so try to make the current content generic.
  * css will not impact only history tab
  * there will be other messages passing, so rename current events
  related to history to be specificly for it
To know if the `localStorage` or `sessionStorage` is impacted.
storage_tool_field_action = Action
storage_tool_field_key = Key
storage_tool_field_time = Time
tool_storage_field_type = Type
Copy link
Contributor

Choose a reason for hiding this comment

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

should be storage_tool_field_type ;)

@dscrobonia
Copy link
Contributor

Hey @Pamplemousse I must be doing something wrong. I'm still not seeing anything in storage:
image

Do i need to do something to install the front-end-tracker as well?

@Pamplemousse
Copy link
Contributor Author

Ah, sorry I did not take the time to give you an update on this...

Having seen the amazing and gigantic work that @jaywon has started in PR #459, I thought it would be smarter for me to wait for a cleaner and refactored structure to build my feature upon rather than hacking it into the current UI.

This is still in a corner of my mind!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants