Revert "Use the virtual target for change events to avoid restoring controlled state on the real target (#10444)" #10504
Conversation
…ontrolled state on the real target (facebook#10444)" This reverts the meaningful (src, non-test) part of commit 3bc6432 since we've reverted the commit it depended on in 18083a8. I don't fully understand the negative implications of leaving this unreverted but it sounds like consensus is that it's safer to revert. I left the new fixture and verified it works correctly in Chrome 62 Mac, as well as the jest tests passing.
In the commit we are reverting, But when I reverted the original PR which removed polyfills, I didn't catch a place where we are no longer passing the fourth argument to A potential quick fix is to also revert Sebastian's commit, such that we no longer expect the fourth argument there, and any other inconsistencies I created would be smoothed out. I will try to verify that either the issue Sebastian was fixing is not present after this fix, or that we reopen and fix the issue. |
|
@jquense The fixture and new unit test added by the commit I'm reverting (but which I'm leaving in) did not pass without the original code change, but they pass after this commit (which includes the revert of your commit). @sebmarkbage also indicated in comments on #10444 that it looked like it blamed to your commit.
@flarnie That's what this PR is – a revert of Sebastian's commit. I verified that after this revert, everything seems to work properly (in particular, the bug that Seb was fixing is not present). |
Right - sorry, I wasn't clear, I was explaining why this commit, which reverts Sebastian's commit, was needed. Thanks for fixing this! If you manually verified then I won't double-dip - lgtm. @jsquense any luck reproducing the bug we discussed in the comments of #10483 ? |
|
Yeah, I verified (included in my original message). :) |
|
Thanks for the explanation! I had missed the signature change. |
4 participants
This reverts the meaningful (src, non-test) part of commit 3bc6432 since we've reverted the commit it depended on in 18083a8. I don't fully understand the negative implications of leaving this unreverted but it sounds like consensus is that it's safer to revert.
I left the new fixture and verified it works correctly in Chrome 62 Mac, as well as the jest tests passing.