-
Notifications
You must be signed in to change notification settings - Fork 419
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
Group popup menu items #653
Conversation
This allows to style specific popup menu.
a9dd9ce
to
b77acf5
Compare
lib/features/popup-menu/PopupMenu.js
Outdated
groupContainer = domQuery('[data-group=' + grouping + ']', entriesContainer); | ||
|
||
if (!groupContainer) { | ||
groupContainer = domify('<div class="group" data-group="' + grouping + '"></div>'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should escape grouping
to not allow XSS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid point. We will need to fix it in the context pad as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now fixed, but there are also a couple of other places where we allow DOM injection. I will fix them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woops woops :).
171f419
to
ec76fdd
Compare
I fixed the existing XSS vulnerabilities via |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great.
Required for bpmn-io/bpmn-js#1682