Bug 1664165 Comment 8 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Summary of the progress so far.

I have added a preference to turn off the webprogress implementation for navigation: marionette.webprogress.enabled.
Even when this preference is enabled, we still use the legacy codepath for click attempts.
The implementation using webprogress listener is using a combination of onStateChange and onLocationChange. 

Challenges/issues:
1. PageLoadStrategy.Eager: onLocationChange is mandatory if we want to replicate the "eager" page load strategy. Otherwise for onStateChange, STATE_START is too early, STATE_STOP is too late. It doesn't seem like webprogresslistener is a good fit for this
1. detecting about:*error redirections is not straightforward. If you attempt to navigate to `"thisprotocoldoesnotexist://"`, you will reach a STATE_STOP for this URL before you are redirected to about:neterror. DevTools detects such bad requests by checking `status != Cr.NS_OK && status != Cr.NS_BINDING_ABORTED` and then waits for the next DOMContentLoaded since in DevTools case this runs in the content process ([searchfox](https://searchfox.org/mozilla-central/rev/5d2b9e940ca09bd1cbc15aa681f69424cde8904c/devtools/server/actors/targets/window-global.js#1922-1930)). Here instead I defer to the next `onLocationChange`, which should be about:neterror. However, there are also seemingly valid requests which match `status != Cr.NS_OK && status != Cr.NS_BINDING_ABORTED`. I had the case for a navigation between an inline image and a regular image ([searchfox](https://searchfox.org/mozilla-central/rev/5d2b9e940ca09bd1cbc15aa681f69424cde8904c/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py#565-566)). So for now I simply wait for some time but if no onLocationChange resolves the navigation, I consider it as completed.
1. I have a lot of intermittent about:blank navigations, although that might be because I was missing the fix from Bug 1747359. Currently rebasing, we'll see if the situation improves.  
1. navigations in driver.js always operate on the top Browsing Context, but navigate.js will watch navigation on the current browsing context. When the current browsing context is an iframe, the behavior is really surprising IMO. The navigation completes because the Frame's browsing context is unloaded and we do listen for that, but I am not sure we are really monitoring the actual full navigation of the top BC. In any case here since we use the webprogresslistener, I think we should always use the top BC

I have a few try pushes in progress which are not too depressingly orange (eg https://treeherder.mozilla.org/jobs?repo=try&revision=ea910686f89a1a17ac891eca713b3d268ac41523), but there's still a lot of work here.
Summary of the progress so far.

I have added a preference to turn off the webprogress implementation for navigation: marionette.webprogress.enabled.
Even when this preference is enabled, we still use the legacy codepath for click attempts.
The implementation using webprogress listener is using a combination of onStateChange and onLocationChange. 

Challenges/issues:
1. PageLoadStrategy.Eager: onLocationChange is mandatory if we want to replicate the "eager" page load strategy. Otherwise for onStateChange, STATE_START is too early, STATE_STOP is too late. It doesn't seem like webprogresslistener is a good fit for this
1. detecting about:*error redirections is not straightforward. If you attempt to navigate to `"thisprotocoldoesnotexist://"`, you will reach a STATE_STOP for this URL before you are redirected to about:neterror. DevTools detects such bad requests by checking `status != Cr.NS_OK && status != Cr.NS_BINDING_ABORTED` and then waits for the next DOMContentLoaded since in DevTools case this runs in the content process ([searchfox](https://searchfox.org/mozilla-central/rev/5d2b9e940ca09bd1cbc15aa681f69424cde8904c/devtools/server/actors/targets/window-global.js#1922-1930)). Here instead I defer to the next `onLocationChange`, which should be about:neterror. However, there are also seemingly valid requests which match `status != Cr.NS_OK && status != Cr.NS_BINDING_ABORTED`. I had the case for a navigation between an inline image and a regular image ([searchfox](https://searchfox.org/mozilla-central/rev/5d2b9e940ca09bd1cbc15aa681f69424cde8904c/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py#565-566)). So for now I simply wait for some time but if no onLocationChange resolves the navigation, I consider it as completed.
1. I have a lot of intermittent about:blank navigations, although that might be because I was missing the fix from Bug 1747359. Currently rebasing, we'll see if the situation improves.  
1. navigations in driver.js always operate on the top Browsing Context, but navigate.js will watch navigation on the current browsing context. When the current browsing context is an iframe, the behavior is really surprising IMO. The navigation completes because the Frame's browsing context is unloaded and we do listen for that, but I am not sure we are really monitoring the actual full navigation of the top BC. In any case here since we use the webprogresslistener, I think we should always use the top BC
1. (edit) overall there seems to be an issue with navigations to pages with inline script tags which open a dialog, eg `inline("<script>window.{}('Hello');</script>".format(dialog_type))`. Usually, this navigation should end when the `common-dialog-loaded` event is received, meaning the consumer can expect to interact with the dialog. However with the webprogresslistener, we sometimes get the STATE_STOP right before before the dialog was opened, and the consumer fails to find the dialog  

I have a few try pushes in progress which are not too depressingly orange (eg https://treeherder.mozilla.org/jobs?repo=try&revision=ea910686f89a1a17ac891eca713b3d268ac41523), but there's still a lot of work here.

Back to Bug 1664165 Comment 8