click and page load aborts too early if target URL loads too slowly (delayed pagehide event)
Categories
(Remote Protocol :: Marionette, defect, P3)
Tracking
(firefox-esr60 wontfix, firefox62 affected, firefox63 affected)
People
(Reporter: whimboo, Unassigned)
References
(Depends on 1 open bug, Blocks 2 open bugs, )
Details
Attachments
(2 files)
Reporter | ||
Updated•7 years ago
|
Comment hidden (offtopic) |
Reporter | ||
Comment 3•6 years ago
|
||
Comment hidden (mozreview-request) |
Reporter | ||
Comment 5•6 years ago
|
||
Comment 6•6 years ago
|
||
Comment 7•6 years ago
|
||
Reporter | ||
Comment 8•6 years ago
|
||
Reporter | ||
Comment 9•4 years ago
|
||
My patch on bug 1612831 will fix this.
Reporter | ||
Comment 10•4 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #9)
My patch on bug 1612831 will fix this.
Note that I actually missed to fix it in that other bug. So lets do it now.
Reporter | ||
Comment 11•4 years ago
|
||
Currently the command only waits 5s for the target page to
be loaded. This needs to be extended to the currently set
page timeout.
Depends on D90413
Reporter | ||
Comment 12•4 years ago
|
||
I forgot about the install_addon case when writing this patch. Extending the timeout to the set page timeout will actually cause a timeout in those cases. See my comment 5 from 2 years ago. That said we need an early return from navigation when such an install notification pops-up. I wonder how best detect it. Gijs, do you know if there is some kind of event that gets fired for newly opened notifications (doorhanger?)?
Comment 13•4 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #12)
I forgot about the install_addon case when writing this patch. Extending the timeout to the set page timeout will actually cause a timeout in those cases. See my comment 5 from 2 years ago. That said we need an early return from navigation when such an install notification pops-up. I wonder how best detect it. Gijs, do you know if there is some kind of event that gets fired for newly opened notifications (doorhanger?)?
There will be a "popupshowing" and "popupshown" event fired on the doorhanger in question. The event bubbles so you can listen for it on the document root and then check the event target to see what is being shown. You probably don't want to do whatever it is you're trying to do for e.g. tooltip
and context menus showing up.
In fact, I also don't understand why you need to return from navigation when a doorhanger pops up... the comments here don't really explain - it sounds to me like the test is just broken and should be fixed, not that navigations should always be canceled by doorhangers. If doorhangers were to always cancel navigation, you'd have lots of other problems (e.g. every time we show a doorhanger, even for completely different things ("we blocked a tracker!", "you can use reader mode here!", "you bookmarked a thing", "you opened a panel while the navigation was happening"), navigation would break).
Reporter | ||
Comment 14•4 years ago
|
||
The problem is that when navigating to URLs with MIME types not handled directly in the tab Marionette currently times out (see bug 1366035), because we only see the beforeunload
and unload
events fired. There is no other event that tells us (for our current navigation listener implementation) that the navigation is done. Maybe not checking for DOMContentLoaded
and pageshow
but using a webprogress listener (bug 1664165) would help? Or is there another way to better detect such situations?
Comment 15•4 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #14)
The problem is that when navigating to URLs with MIME types not handled directly in the tab
Can you give a concrete way of reproducing this / example of what you mean?
Marionette currently times out (see bug 1366035), because we only see the
beforeunload
andunload
events fired.
This doesn't really make sense - if something doesn't load in the tab, it shouldn't trip an "unload" event, because the current document should stay loaded.
There is no other event that tells us (for our current navigation listener implementation) that the navigation is done. Maybe not checking for
DOMContentLoaded
andpageshow
but using a webprogress listener (bug 1664165) would help? Or is there another way to better detect such situations?
Putting more of this in the parent process using webprogress listeners does seem like it'd be a good idea regardless.
What would marionette actually want to happen when an attempt to navigate does not result in the tab loading a new page (instead e.g. opening an external app via a protocol, or a file type we can't handle, or a file type we handle by offering to install an add-on, or whatever else we support) ?
Reporter | ||
Comment 16•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #15)
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #14)
The problem is that when navigating to URLs with MIME types not handled directly in the tab
Can you give a concrete way of reproducing this / example of what you mean?
There is the following test for installing an addon by clicking a link:
Here appropriate log output:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=316700896&repo=mozilla-central&lineNumber=7448-7457
Marionette currently times out (see bug 1366035), because we only see the
beforeunload
andunload
events fired.This doesn't really make sense - if something doesn't load in the tab, it shouldn't trip an "unload" event, because the current document should stay loaded.
Sorry, that was my fault. We indeed don't see a unload
event but only beforeunload
. See the above referenced log output.
There is no other event that tells us (for our current navigation listener implementation) that the navigation is done. Maybe not checking for
DOMContentLoaded
andpageshow
but using a webprogress listener (bug 1664165) would help? Or is there another way to better detect such situations?Putting more of this in the parent process using webprogress listeners does seem like it'd be a good idea regardless.
What would marionette actually want to happen when an attempt to navigate does not result in the tab loading a new page (instead e.g. opening an external app via a protocol, or a file type we can't handle, or a file type we handle by offering to install an add-on, or whatever else we support) ?
For unknown file types that we cannot handle we might have to install an override handler, right? When we hit that the navigation should stop. But when installing an add-on there is no such dialog that we can set a handler for, but the notifications.
I assume the above situations might be similar to when a beforeunload
event gets rejected and the navigation isn't going to happen. In such a case we might also have to stop waiting. The Webdriver spec defines the following to wait for the navigation to complete:
https://w3c.github.io/webdriver/#dfn-waiting-for-the-navigation-to-complete
But the above cases aren't covered here and would need special treatment for Firefox specific behavior.
Not sure if it would make sense to set a timeout for unload
and if that doesn't happen within a specific time also return from the navigation request. It feels a bit racy to me similar to what we do with beforeunload
.
Comment 17•4 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #16)
For unknown file types that we cannot handle we might have to install an override handler, right?
I don't know; I think you may be able to deal with it using just web progress listeners. But that'd need some testing.
When we hit that the navigation should stop. But when installing an add-on there is no such dialog that we can set a handler for, but the notifications.
Yeah.
I assume the above situations might be similar to when a
beforeunload
event gets rejected and the navigation isn't going to happen. In such a case we might also have to stop waiting.
Yes. I'd expect that webprogress listeners should notify you of a start and then stop of a load (or perhaps never even register the start, but either way, state should be consistent).
The Webdriver spec defines the following to wait for the navigation to complete:
https://w3c.github.io/webdriver/#dfn-waiting-for-the-navigation-to-complete
But the above cases aren't covered here and would need special treatment for Firefox specific behavior.
I mean, other browsers also support downloading files, right? The HTML spec lists what happens at https://html.spec.whatwg.org/#process-a-navigate-response . I don't know how webdriver is supposed to handle this.
Reporter | ||
Comment 18•4 years ago
|
||
I'm currently not working on this bug. Also given the reply from Gijs it might be good to wait until we make use of a webprogress listener for tracking an active navigation. See bug 1664165.
Updated•2 years ago
|
Updated•2 years ago
|
Description
•