-
Notifications
You must be signed in to change notification settings - Fork 20.6k
Fixes to resolve unsafe warnings in WWAs #1537
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
Conversation
Setting the innerHTML property in an unsafe manner raises issues in Windows Web Applications. Strings being passed into innerHTML cannot include name the attribute. Creating elements, and settings attributes, has far better cross-environment compliance.
Thanks for your contribution! This will need comments like "Support: Windows Web Apps" (did we name it differently in other parts of the code base?) to justify this madness so that someone doesn't remove it accidentally. It's bad we don't have a way to automatically test it. Do we have contacts in MS we could signal it's really important to us? cc @dmethvin |
option.setAttribute("selected", ""); | ||
|
||
select.appendChild(option); | ||
div.appendChild(select); |
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.
Sizzle changes must be addressed in Sizzle.
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.
Ugh, I was wondering why this cropped up since we were passing in WWA at one point. It was changed in 6d2c5c7. @timmywil do you recall if this was causing a test fail for 5.1? Or was it just an optimization? If it's just the latter I can land something like what @jonathansampson is proposing and add a comment about not using |
Oh also I'm wondering if we even need this specific part of the feature test, per the comment from @mzgol in http://bugs.jquery.com/ticket/11217 . This patch seems to predate our |
@dmethvin I believe we were getting false positives if |
Fixes jquerygh-2501 Closes jquerygh-2504 Refs jquerygh-1950 Refs jquerygh-1949 Refs jquerygh-2397 Refs jquerygh-1537 Refs 842958e
Fixes jquerygh-2133 Fixes jquerygh-2501 Closes jquerygh-2504 Refs jquerygh-1950 Refs jquerygh-1949 Refs jquerygh-2397 Refs jquerygh-1537 Refs 842958e
Fixes jquerygh-2133 Fixes jquerygh-2501 Closes jquerygh-2504 Refs jquerygh-1950 Refs jquerygh-1949 Refs jquerygh-2397 Refs jquerygh-1537 Refs jquerygh-2504 Refs 842958e
Fixes jquerygh-2133 Fixes jquerygh-2501 Closes jquerygh-2504 Refs jquerygh-1950 Refs jquerygh-1949 Refs jquerygh-2397 Refs jquerygh-1537 Refs jquerygh-2504 Refs 842958e
Setting the innerHTML property in an unsafe manner raises issues in
Windows Web Applications. Strings being passed into innerHTML cannot
include name the attribute. Creating elements, and settings attributes,
has far better cross-environment compliance.
Historical context: