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

Improve DebugLog data sanitation #1353

Merged
merged 1 commit into from
Jan 12, 2017

Conversation

AFaust
Copy link
Contributor

@AFaust AFaust commented Jan 11, 2017

I am currently tracking down an issue with infinite scrolling on an AlfFilteredList in an Aikau-ified Surf component (not a hybrid or full page web script - in this case I am replacing the people-finder component in the page of that name via a sub-component) using the InfiniteScrollingService. During debugging I noticed that immediately after the initial page load and a single scroll action, the PubSubLog was extremely slow to open. Looking for ALF_EVENTS_SCROLL revealed that the payload published contains a reference to the window object which is currently not detected in the sanitation code. As a result, the entire window object and all the data accessible through it have been recursively processed. This even included data from localStorage, which - among other data - contained a very large script I recently executed via the JavaScript console.

This PR adds an extra check to avoid processing the window object. Additionally, this PR improves on the check for widget instances - the _attachPoints previously checked originate from the _AttachMixin that not all widgets may have mixed in (at PRODYNA I avoided that mixin due to the overhead it introduced in complex widget structures). Finally, I added a check for any services that may be reference in payload in a similar manner to the check for widget instances.

Apply these changes to my instance dramatically improved the "snappiness" of the PubSubLog after long durations of scroll testing.

@draperd
Copy link

draperd commented Jan 12, 2017

Thanks for creating this PR... I'll check out the branch and run the regression tests because obviously the output of the DebugLog is essential to the unit tests. If the unit tests pass then I'll merge this.

@draperd
Copy link

draperd commented Jan 12, 2017

Regression tests pass, merging

@draperd draperd merged commit 0286a8c into Alfresco:develop Jan 12, 2017
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.

2 participants