Closed benjaminpkane closed 2 weeks ago
The recent code updates focus on enhancing security and performance by replacing the use of innerHTML
with textContent
across various elements. Additionally, new utility functions and minor state module imports adjustments were made, refining the code structure and functionality. These changes enhance text handling safety by mitigating potential injection attacks without altering the elements' core functionalities.
File Path | Change Summary |
---|---|
app/packages/looker/src/elements/common/error.ts |
Changed innerHTML to textContent for videoText in ErrorElement . Removed StateUpdate import. |
app/packages/looker/src/elements/common/tags.ts |
Extracted function for tag element creation. Changed innerHTML to textContent for TagsElement . |
app/packages/looker/src/elements/common/util.ts |
Changed innerHTML to textContent for labels and anchor elements in utility functions. |
app/packages/looker/src/elements/frame.ts |
Updated FrameNumberElement to use textContent instead of innerHTML . |
app/packages/looker/src/elements/imavid/frame-count.ts |
Changed innerHTML to textContent for FrameCountElement . |
app/packages/looker/src/elements/imavid/play-button.ts |
Updated PlayButtonElement to use textContent instead of innerHTML . |
app/packages/looker/src/elements/video.ts |
Changed innerHTML to textContent for PlayButtonElement and TimeElement classes. |
Amidst the code, a shift took place,
Text content found its safer space.
HTML's charm we bade farewell,
Secure and clean, our projects swell.
With every change, a better day,
CodeRabbit's touch, hips, hooray!
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Hmm... my comment was duplicated and then I deleted one which deleted both
But the response https://github.com/voxel51/fiftyone/pull/4497#discussion_r1640112738 here was to comment:
I prefer Ben's original implementation over the suggestion as it's easier to read
What changes are proposed in this pull request?
Replaces
element.innerHTML
usage withelement.textContent
. In particular, Looker tags now use anappleTagValue
function that prevents XSSHow is this patch tested? If it is not, please explain why.
Unit test
Release Notes
What areas of FiftyOne does this PR affect?
fiftyone
Python library changesSummary by CodeRabbit
Refactor
innerHTML
withtextContent
for various elements across multiple components.Chores
StateUpdate
import to clean up codebase.