-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Fix #17964 #17997
Fix #17964 #17997
Conversation
Some browsers are lazy when updating dom elements after transition effects. This can be fixed by reading element properties such as offsetHeight or offsetWidth. However, creating a function using the Function constructor just to access such element, results in a violation of Content Security Policy (where applied), which in turn crashes the application. This fix actually reverts to the way this was handled in v3 and should work as intended.
@@ -116,7 +116,7 @@ const Util = (($) => { | |||
}, | |||
|
|||
reflow(element) { | |||
new Function('bs', 'return bs')(element.offsetHeight) | |||
return element.offsetHeight |
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.
Missing semicolon.
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.
Semicolon is forbidden.
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.
It is necessary to adjust the hound...
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.
javascript
write without _
(undescore) https://houndci.com/configuration#javascript.
I think java_script
is ignore in .hound.yml
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, fixed in 18ad53f
Misguided by travisbot...
Per #17997 (comment) ; thanks @vsn4ik ! Refs #17769 /fyi @croaky [ci skip]
Per #17997 (comment) ; thanks @vsn4ik ! Refs #17769 /fyi @croaky [ci skip]
Curious if I could get some eyes on this from other folks who've pushed JS changes—@cvrebert, @Johann-S, or @bardiharborow. Know it's a small change, but I don't know the full context here <3. |
I actually did see this in the code and was wondering what we should do about it. The concern I have is that it was probably written in such a novel way because of a browser bug and I would want to ensure that it isn't still necessarily. So far I've traced it back to 834220e, but there is little indication why it was added. |
I'll punt it to Alpha 6 until we know more then. We can always ping @fat to see if it needs to stay the same, but if this is coming from that Closure commit, I'm willing to bet it's not a weird browser thing. Thanks for looking! |
I've found some references to this technique on StackExchange, though they don't use |
Okay, so maybe we punt on this for now then @bardiharborow? |
@mdo, I'd say merge this. The Function wrapper seems pointless based on everything I've read, it's more to stop Closure from being too smart and removing this entirely. The |
Per twbs#17997 (comment) ; thanks @vsn4ik ! Refs twbs#17769 /fyi @croaky [ci skip]
Some browsers are lazy when updating dom elements after transition effects. This can be fixed by reading element properties such as offsetHeight or offsetWidth, which causes the browser to resync (reflow) the element.
However, creating a function using the Function constructor just to access such element properties, results in a violation of Content Security Policy (where applied), which in turn crashes the application. This fix actually reverts to the way this was handled in v3 and should work as intended.