Bug 94471

Summary: [GTK] Replace webkit_web_view_replace_content with webkit_web_view_load_alternate_html
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo, gyuyoung.kim, mrobinson, webkit.review.bot, xan.lopez
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch mrobinson: review+

Carlos Garcia Campos
Reported 2012-08-20 05:35:23 PDT
In WebKit1 there's code to prevent that load signals are emitted when loading internal custom error pages. We added webkit_web_view_replace_content() with the same idea, but it has ended up being problematic, because it allows to add any HTML content (in WebKit1 we assumed internal error pages never fails and always load) and it's impossible to know when the load has finished. It also required a lot of logic to handle replace_content as an especial case, in order to hide the fact that it loads content. So, let's rename it to webkit_web_view_load_alternate_html() and emit load events normally. Clients can easily ignore load signals when loading alternate content.
Attachments
Patch (23.62 KB, patch)
2012-08-20 05:40 PDT, Carlos Garcia Campos
mrobinson: review+
Carlos Garcia Campos
Comment 1 2012-08-20 05:40:26 PDT
WebKit Review Bot
Comment 2 2012-08-20 05:42:14 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Martin Robinson
Comment 3 2012-08-20 07:22:50 PDT
Comment on attachment 159403 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159403&action=review Thanks! > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1554 > + * error page, the the back-forward list is maintained appropriately. Hrm. Would it make sense to just say outright something like, "When this method is called, a new entry is added to the back-forward list." ? > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1555 > + * For everything else this method works the same way than webkit_web_view_load_html(). Nit: same way than -> same way as
Carlos Garcia Campos
Comment 4 2012-08-20 08:07:10 PDT
(In reply to comment #3) > (From update of attachment 159403 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=159403&action=review > > Thanks! > > > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1554 > > + * error page, the the back-forward list is maintained appropriately. > > Hrm. Would it make sense to just say outright something like, "When this method is called, a new entry is added to the back-forward list." ? It's more complicated than that, when alternate string is called from a load error callback, going back/forward from a load error page convert the load into a reload to avoid a new time to be added to the back forward list. See this bug: https://bugs.webkit.org/show_bug.cgi?id=94476 Because it's currently broken in wk2, and see the implementation here: http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp So, I copied the description from the wk1 mac docs: https://developer.apple.com/library/mac/#documentation/Cocoa/Reference/WebKit/ObjC_classic/_index.html I plan to add a unit test for that case, skipped until bug #94476 is fixed. > > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1555 > > + * For everything else this method works the same way than webkit_web_view_load_html(). > > Nit: same way than -> same way as Ok.
Carlos Garcia Campos
Comment 5 2012-08-20 08:11:18 PDT
Note You need to log in before you can comment on or make changes to this bug.