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
Escape sourceLabel ids when invalid characters are present #3351
Conversation
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.
I think that's better to have a function that return the escaped id and called here: https://github.com/camptocamp/ngeo/blob/2.2/contribs/gmf/src/directives/partials/displayquerygrid.html#L41
d32761c
to
ba47927
Compare
@ger-benjamin I refactored the code. Now it is escaped at data generation (before the function that going to fill the grid). I did the change to use Ids again in the HTML instead of the labels. Labels are still used for the tab labels, so nothing change for the user. |
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.
Thanks.
Looks good but please, test carefully before to merge.
/** | ||
* Returns an escaped value. | ||
* @param {string|number} value A value to escape. | ||
* @returns {string|number} value An escaped valjue. |
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.
typo
6b704fc
to
e8e42dd
Compare
e8e42dd
to
bc2ae5a
Compare
@ger-benjamin I added two merged tabs in desktop_alt to make sure we have correct testing setup. |
Fixes #3273.
With this fix, we escape the IDs (generated from the value in ctrl.mergeTabs for merged tabs or OL datasource). If an invalid value would result in an invalid ID selector (css), then the value is stripped from unauthorized characters. Other change is the use of the datasource IDs for the HTML ids instead of the label value that contained spaces, etc.
Ideally ids defined in the ctrl.mergeTabs should have no spaces, or any specific characters such as parentheses, brackets, etc. This fix allow all project to work without changing ids manually, but if possible having a correct value is just best practice...
In that context I changed the value in our desktop example, while the fix was tested on the previous value, 'OSM time merged'.