Summary: | [GTK] Running minibrowser with url crashes in debug build | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jae Hyun Park <jaepark> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | berto, cgarcia, commit-queue, fvallee, gustavo, mario, mrobinson, svillar | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Jae Hyun Park
2014-03-19 18:49:47 PDT
Created attachment 228051 [details]
Patch proposal
This patch fixes the issue by using SoupURI API to create a URL object to be passed to loadRequest()
Please review
*** Bug 129606 has been marked as a duplicate of this bug. *** 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 Comment on attachment 228051 [details]
Patch proposal
Thanks!
Comment on attachment 228051 [details] Patch proposal Clearing flags on attachment: 228051 Committed r166410: <http://trac.webkit.org/changeset/166410> All reviewed patches have been landed. Closing bug. This broke /webkit2/WebKitWebContext/uri-scheme /webkit2/WebKitWebContext/uri-scheme: ** ERROR:../../Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:30:void loadChangedCallback(WebKitWebView*, WebKitLoadEvent, LoadTrackingTest*): assertion failed (test->m_activeURI.data() == webkit_web_view_get_uri(webView)): ("echo:hello world" == "echo:hello%20world") (In reply to comment #7) > This broke /webkit2/WebKitWebContext/uri-scheme > > /webkit2/WebKitWebContext/uri-scheme: ** > ERROR:../../Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:30:void loadChangedCallback(WebKitWebView*, WebKitLoadEvent, LoadTrackingTest*): assertion failed (test->m_activeURI.data() == webkit_web_view_get_uri(webView)): ("echo:hello world" == "echo:hello%20world") I think the problem here is in the test, not in the code, since "hello world" is not a valid URI (it should be normalized to "hello%20world" to become one) and so that's why the WebView, after this patch landed, is returning a mismatch in comparison to what the test class has stored. So, unless there's a good reason to support URIs such as "hello world" or "foo bar" in this test, I'd suggest to just update the test to use "hello-world" or "foo-bar" in these two cases, so that we do not need to either artificially unnormalize whatever valid URI comes from webkit_web_view_get_uri() nor to normalize the URIs used in these test cases. (In reply to comment #8) > (In reply to comment #7) > > This broke /webkit2/WebKitWebContext/uri-scheme > > > > /webkit2/WebKitWebContext/uri-scheme: ** > > ERROR:../../Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:30:void loadChangedCallback(WebKitWebView*, WebKitLoadEvent, LoadTrackingTest*): assertion failed (test->m_activeURI.data() == webkit_web_view_get_uri(webView)): ("echo:hello world" == "echo:hello%20world") > > I think the problem here is in the test, not in the code, since "hello world" is not a valid URI (it should be normalized to "hello%20world" to become one) and so that's why the WebView, after this patch landed, is returning a mismatch in comparison to what the test class has stored. Yes, I think you are right. > So, unless there's a good reason to support URIs such as "hello world" or "foo bar" in this test, I'd suggest to just update the test to use "hello-world" or "foo-bar" in these two cases, so that we do not need to either artificially unnormalize whatever valid URI comes from webkit_web_view_get_uri() nor to normalize the URIs used in these test cases. Sounds good to me Created attachment 228177 [details]
Patch proposal
Patch updating the test case as mentioned in the previous comment. Please review
Comment on attachment 228177 [details]
Patch proposal
Thanks for the review. Now setting the cq+ flag
Comment on attachment 228177 [details]
Patch proposal
This cq+ did not work. Applying manually...
Committed r166577: <http://trac.webkit.org/changeset/166577> |