Tracking PR for our fork #21
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instruct composer that it replaces dmore/chrome-mink-driver cherry-picks 91e4437
Cherry-picks a number of commits that went through over time: * b4357d7 * 326e0a6 * 326e0a6 * fbf85be * 89822e4
Reset our fork to latest upstream Replaces #17
Squashed commit of the following: commit 69658a3 Author: acoulton <andrew@ingenerator.com> Date: Wed May 11 16:33:43 2022 +0100 Remove handler for Security.certificateError DevTools has deprecated the old flow of setting overrideCertificateErrors and then handling an event notification. Instead they are just ignored upfront. The driver was migrated to the new flow in 457160a in 2018, but this (now-dead) code was not removed at the time. These events will not have been arriving since then, but leaves another place in the code where it looks like the page state might change. commit 8bd398f Author: acoulton <andrew@ingenerator.com> Date: Wed May 11 16:26:20 2022 +0100 Don't take `Animation` events, they aren't actually used Animation events produce a flood of events on the dev-tools protocol particularly (in many projects) around the time of click, page-load etc events. This may be contributing to backlogging the socket and the driver occasionally missing a more important page completion etc event. The rate of animation events is particularly high because we are also setting a massive animation playback rate so animations start and stop very quickly. It is therefore virtually impossible to actually interact with an element during an animation. The code *looks* like it is sleeping when animations begin - this was introduced in b023983 and "fixed" in 0b8113f. However, in practice that fix actually just disabled the sleep because the parameter from DevTools is actually in `$data['params']['animation']['source']['duration']` (note the extra `animation` in the path). So the `if (!empty(` has always been returning false and the driver has never actually been running any code in response to these events. If there was a desire / need to reintroduce sleeping during animations in future, we'd need to review the units of the `duration` value, and whether just multiplying that by 10 is even close to the actual time taken when playing back at the forced accelerated rate. commit 7106c4b Author: acoulton <andrew@ingenerator.com> Date: Tue May 10 14:40:11 2022 +0100 Only reset request headers after scenario if they were set Setting manual request headers on a Chrome instance seems somewhat unlikely, but it is exposed by the Mink interface and implemented by the driver so they do need to be cleaned up as part of ->reset() if present. However, if the browser or devtools connection has crashed / hit a problemn then calling Network.setExtraHttpHeaders can fail in a way that masks and/or makes it harder to trace the actual failure. Therefore, avoid sending this call unless it is actually required (e.g. some custom headers have been set since the session started or was last reset). commit 74528a2 Author: acoulton <andrew@ingenerator.com> Date: Mon May 9 23:13:53 2022 +0100 Treat a `null` websocket return as an exception Previously this was silently skipping out of the waitFor loop, I don't think that can be right. There's no comment or information in the code or commit history to indicate why this was the case - it has been there ever since the very first commit. Possibly it was originally meant to be a `continue`? Even if we do encounter this happening, it seems entirely wrong to allow the calling code to continue without waiting for any further events or any indication that the thing it was actually waiting for has happened. Therefore throw an exception and we can then explore if / when / how to deal with it once we know when & why it happens.
Squashed commit of the following: commit fd1a98b Author: acoulton <andrew@ingenerator.com> Date: Tue May 17 10:13:25 2022 +0100 Natively flush Chrome localstorage for the domain Now that the driver always starts on about:blank, it's not possible to use javascript to flush localstorage from a hook because it's not on the right domain. Fortunately devtools protocol exposes a command for this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To keep our main branch in sync with upstream, with our changes merged on top.