-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Use modern DOM methods a bit more (PR 15031 follow-up) #15035
Use modern DOM methods a bit more (PR 15031 follow-up) #15035
Conversation
Apparently the ESLint rule added in PR 15031 wasn't able to catch all cases that can be converted, which is probably not all that surprising given how some of these call-sites look. - Use `Element.prepend()` to insert nodes before all other ones in the element, rather than using `firstChild` with `insertBefore`-calls; see https://developer.mozilla.org/en-US/docs/Web/API/Element/prepend - Fix one *incorrect* `insertBefore` call, in the AnnotationLayer-code. Initially the patch simply changed that to an `Element.before()`-call, however that broke one of the integration-tests. It turns out that the `index` may try to access a non-existent select-child, which triggers undefined behaviour; note the warning in https://developer.mozilla.org/en-US/docs/Web/API/Node/insertBefore#parameters
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/d0c1ba57aebfb83/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/f82ac5eb4dcd447/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/f82ac5eb4dcd447/output.txt Total script time: 26.04 mins
Image differences available at: http://54.241.84.105:8877/f82ac5eb4dcd447/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/d0c1ba57aebfb83/output.txt Total script time: 28.11 mins
Image differences available at: http://54.193.163.58:8877/d0c1ba57aebfb83/reftest-analyzer.html#web=eq.log |
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.
Clearly better like this.
Apparently the ESLint rule added in PR #15031 wasn't able to catch all cases that can be converted, which is probably not all that surprising given how some of these call-sites look.
Use
Element.prepend()
to insert nodes before all other ones in the element, rather than usingfirstChild
withinsertBefore
-calls; see https://developer.mozilla.org/en-US/docs/Web/API/Element/prependFix one incorrect
insertBefore
call, in the AnnotationLayer-code.Initially the patch simply changed that to an
Element.before()
-call, however that broke one of the integration-tests. It turns out that theindex
may try to access a non-existent select-child, which triggers undefined behaviour; note the warning in https://developer.mozilla.org/en-US/docs/Web/API/Node/insertBefore#parameters