Summary: | [GTK] No way to safely use webkit_web_view_get_snapshot() as pages are not rendered when LOAD_FINISHED is called | ||
---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> |
Status: | NEW --- | ||
Severity: | Normal | CC: | annulen, aperez, bugs-noreply, calvaris, cgarcia, clopez, mcatanzaro, tpopela |
Priority: | P2 | ||
Version: | Other | ||
Hardware: | PC | ||
OS: | Linux | ||
See Also: | https://bugzilla.gnome.org/show_bug.cgi?id=787486 |
Description
Michael Catanzaro
2016-10-29 08:36:27 PDT
I don't think we should change the semantics of LOAD_FINISHED, we should probably expose APILoaderClient::didReachLayoutMilestone, and the applications could wait for DidFirstVisuallyNonEmptyLayout or DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the milestones. (In reply to comment #1) > I don't think we should change the semantics of LOAD_FINISHED, we should > probably expose APILoaderClient::didReachLayoutMilestone, and the > applications could wait for DidFirstVisuallyNonEmptyLayout or > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the > milestones. Why you think we should not change when LOAD_FINISHED triggers? What is the use case of having a LOAD_FINISHED signal that doesn't trigger always when the page is fully rendered?? (In reply to comment #2) > (In reply to comment #1) > > I don't think we should change the semantics of LOAD_FINISHED, we should > > probably expose APILoaderClient::didReachLayoutMilestone, and the > > applications could wait for DidFirstVisuallyNonEmptyLayout or > > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the > > milestones. > > Why you think we should not change when LOAD_FINISHED triggers? > What is the use case of having a LOAD_FINISHED signal that doesn't trigger > always when the page is fully rendered?? Because load finished is not about rendering but about networking, the page finished loading, but not necessarily rendering. That's why there are other callbacks more related to rendering like the ones I mentioned before. (In reply to comment #3) > (In reply to comment #2) > > (In reply to comment #1) > > > I don't think we should change the semantics of LOAD_FINISHED, we should > > > probably expose APILoaderClient::didReachLayoutMilestone, and the > > > applications could wait for DidFirstVisuallyNonEmptyLayout or > > > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the > > > milestones. > > > > Why you think we should not change when LOAD_FINISHED triggers? > > What is the use case of having a LOAD_FINISHED signal that doesn't trigger > > always when the page is fully rendered?? > > Because load finished is not about rendering but about networking, the page > finished loading, but not necessarily rendering. That's why there are other > callbacks more related to rendering like the ones I mentioned before. I think that this is an internal implementation detail that don't necessarily has to be exposed in our API Quoting our docs: " WEBKIT_LOAD_FINISHED This state means that everything that was required to display the page has been loaded. " https://webkitgtk.org/reference/webkitgtk/stable/WebKitWebFrame.html#WebKitLoadStatus And after reading that twice, I would expect that when LOAD_FINISHED triggers the page is always fully rendered. Adding a new state is an option, but I think is a bad idea to do it, if we only add it for the sake of exposing the internal implementation. So, what is the use case for an application to have both WEBKIT_LOAD_FINISHED and WEBKIT_LOAD_FINISHED_AND_RENDERED_COMPLETED ? (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (In reply to comment #1) > > > > I don't think we should change the semantics of LOAD_FINISHED, we should > > > > probably expose APILoaderClient::didReachLayoutMilestone, and the > > > > applications could wait for DidFirstVisuallyNonEmptyLayout or > > > > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the > > > > milestones. > > > > > > Why you think we should not change when LOAD_FINISHED triggers? > > > What is the use case of having a LOAD_FINISHED signal that doesn't trigger > > > always when the page is fully rendered?? > > > > Because load finished is not about rendering but about networking, the page > > finished loading, but not necessarily rendering. That's why there are other > > callbacks more related to rendering like the ones I mentioned before. > > I think that this is an internal implementation detail that don't > necessarily has to be exposed in our API > > Quoting our docs: > > " WEBKIT_LOAD_FINISHED This state means that everything that was required to > display the page has been loaded. " > > https://webkitgtk.org/reference/webkitgtk/stable/WebKitWebFrame. > html#WebKitLoadStatus Sorry, I got the doc from the webkit1 API by mistake. The one for WK2: WEBKIT_LOAD_FINISHED Load completed. All resources are done loading or there was an error during the load operation. https://webkitgtk.org/reference/webkit2gtk/stable/WebKitWebView.html#WebKitLoadEvent My understanding of what that means remains the same. (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (In reply to comment #1) > > > > I don't think we should change the semantics of LOAD_FINISHED, we should > > > > probably expose APILoaderClient::didReachLayoutMilestone, and the > > > > applications could wait for DidFirstVisuallyNonEmptyLayout or > > > > DidHitRelevantRepaintedObjectsAreaThreshold, I don't know the details of the > > > > milestones. > > > > > > Why you think we should not change when LOAD_FINISHED triggers? > > > What is the use case of having a LOAD_FINISHED signal that doesn't trigger > > > always when the page is fully rendered?? > > > > Because load finished is not about rendering but about networking, the page > > finished loading, but not necessarily rendering. That's why there are other > > callbacks more related to rendering like the ones I mentioned before. > > I think that this is an internal implementation detail that don't > necessarily has to be exposed in our API > > Quoting our docs: > > " WEBKIT_LOAD_FINISHED This state means that everything that was required to > display the page has been loaded. " Everything required to display doesn't mean it has displayed eferything, only that the resources are there, but you have to parse, layout, paint, etc. > https://webkitgtk.org/reference/webkitgtk/stable/WebKitWebFrame. > html#WebKitLoadStatus > > And after reading that twice, I would expect that when LOAD_FINISHED > triggers the page is always fully rendered. > > Adding a new state is an option, but I think is a bad idea to do it, if we > only add it for the sake of exposing the internal implementation. > > So, what is the use case for an application to have both > WEBKIT_LOAD_FINISHED and WEBKIT_LOAD_FINISHED_AND_RENDERED_COMPLETED ? I don't think it's possible to know when a page has finished rendering, you can know if something has rendered, or if there was a layout, etc. But, if page has dynamic contents or animations. Load events has to do with the load, what happens in WebCore/loader, not with the layout or render. If we want to expose notifications about those I would not add new load events, but a different signal or whatever I didn't know about that bug and I created bug 168830. The thing is basically the same. We should fix webkit_web_view_get_snapshot() and printing to internally wait until the layout has finished if possible. As Michael said there is a nasty workaround for webkit_web_view_get_snapshot(), but that one could not be applied for the situation in the bug 168830. *** Bug 168830 has been marked as a duplicate of this bug. *** Same problem exists with zoom, there's no way for the browser to know the right time to call webkit_web_view_set_zoom_level(). When porting the GIMP's “web-page” plug-in to use modern WebKitGTK+ we have hit this issue as well, as per the comments at: https://gitlab.gnome.org/GNOME/gimp/merge_requests/15 It seems that most tools out there are artificially adding a timer delay before attempting to save snapshots, like for example the “webkit-capture” Python script which I have used sometimes: https://github.com/nud/webkit-capture/blob/e67858eef6265050ffa36644e2ac4b00862538fc/webkit-capture#L158-L160 Having this solved would be nice. I agree that conceptually it does not make sense to use the WebKitWebView::load-changed signal to notify about layout completion, because this one is about *network* activity. Wouldn't it be acceptable to have a new WebKitWebView::layout-completed signal? Adding a signal would not break the API/ABI, which would remain backwards-compatible. (In reply to Adrian Perez from comment #10) > Wouldn't it be acceptable to have a new WebKitWebView::layout-completed > signal? Adding a signal would not break the API/ABI, which would remain > backwards-compatible. That might be safer than exposing all the details of APILoaderClient::didReachLayoutMilestone. |