After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 683100 - Use custom URI schemes to handle document URIs
Use custom URI schemes to handle document URIs
Status: RESOLVED FIXED
Product: yelp
Classification: Applications
Component: General
git master
Other Linux
: Normal normal
: ---
Assigned To: Yelp maintainers
Yelp maintainers
Depends on:
Blocks: 686376
 
 
Reported: 2012-08-31 12:34 UTC by Carlos Garcia Campos
Modified: 2015-06-24 11:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
yelp-document: Make document-uri property a YelpUri (20.82 KB, patch)
2012-08-31 12:35 UTC, Carlos Garcia Campos
committed Details | Review
yelp-mallard-document: Fix use of uninitialized variable (1.16 KB, patch)
2012-08-31 12:36 UTC, Carlos Garcia Campos
committed Details | Review
yelp-document: Return HTML mime type insted of NULL for search pages (827 bytes, patch)
2012-08-31 12:37 UTC, Carlos Garcia Campos
committed Details | Review
yelp-document: Add yelp_document_get_for_docuri() (1.77 KB, patch)
2012-08-31 12:37 UTC, Carlos Garcia Campos
needs-work Details | Review
yelp-common.xsl: Use a full file:// URI for the javascript (940 bytes, patch)
2012-08-31 12:38 UTC, Carlos Garcia Campos
none Details | Review
yelp-view: Fix memory leaks (929 bytes, patch)
2012-08-31 12:39 UTC, Carlos Garcia Campos
committed Details | Review
yelp-document: Mutex is unlocked twice when generating search results (874 bytes, patch)
2012-08-31 12:40 UTC, Carlos Garcia Campos
committed Details | Review
yelp-document: Use xref: uris for search results (1.56 KB, patch)
2012-08-31 12:41 UTC, Carlos Garcia Campos
committed Details | Review
yelp-uri: Use 'index' as default page for help: uris when resolving a xref (963 bytes, patch)
2012-08-31 12:42 UTC, Carlos Garcia Campos
committed Details | Review
yelp-view: Don't free the passed GError in view_show_error_page() (1.97 KB, patch)
2012-08-31 12:43 UTC, Carlos Garcia Campos
committed Details | Review
yelp-view: Use custom URI schemes to handle document URIs (42.95 KB, patch)
2012-08-31 12:44 UTC, Carlos Garcia Campos
none Details | Review
yelp-document: Add yelp_document_lookup_document_uri() (15.36 KB, patch)
2012-11-22 13:19 UTC, Carlos Garcia Campos
committed Details | Review
yelp-view: Use custom URI schemes to handle document URIs (42.94 KB, patch)
2012-11-22 13:20 UTC, Carlos Garcia Campos
none Details | Review
Rebase of the YelpUri patch (23.48 KB, patch)
2014-09-19 07:49 UTC, Marcos Chavarria Teijeiro
none Details | Review
Rebase of the yelp_document_lookup_document_uri patch (15.34 KB, patch)
2014-09-19 07:50 UTC, Marcos Chavarria Teijeiro
none Details | Review
"return html mimetype" patch rebase (1.32 KB, patch)
2014-10-31 12:09 UTC, Marcos Chavarria Teijeiro
none Details | Review

Description Carlos Garcia Campos 2012-08-31 12:34:15 UTC
Instead of passing the HTML data directly to webkit using webkit_web_view_load_html_string(), we could implement custom URI schemes handlers for help URIs (help: ghelp: etc.) and use webkit_web_view_load_uri(). This way we don't need to change the URI of resources that are being loaded. This is required to port yelp to WebKit2, because in WebKit2 it's not possible to change the URI of a resource network request when it's loaded.
Comment 1 Carlos Garcia Campos 2012-08-31 12:35:17 UTC
Created attachment 223045 [details] [review]
yelp-document: Make document-uri property a YelpUri
Comment 2 Carlos Garcia Campos 2012-08-31 12:36:19 UTC
Created attachment 223046 [details] [review]
yelp-mallard-document: Fix use of uninitialized variable
Comment 3 Carlos Garcia Campos 2012-08-31 12:37:10 UTC
Created attachment 223047 [details] [review]
yelp-document: Return HTML mime type insted of NULL for search pages
Comment 4 Carlos Garcia Campos 2012-08-31 12:37:58 UTC
Created attachment 223048 [details] [review]
yelp-document: Add yelp_document_get_for_docuri()
Comment 5 Carlos Garcia Campos 2012-08-31 12:38:46 UTC
Created attachment 223049 [details] [review]
yelp-common.xsl: Use a full file:// URI for the javascript
Comment 6 Carlos Garcia Campos 2012-08-31 12:39:41 UTC
Created attachment 223050 [details] [review]
yelp-view: Fix memory leaks
Comment 7 Carlos Garcia Campos 2012-08-31 12:40:33 UTC
Created attachment 223051 [details] [review]
yelp-document: Mutex is unlocked twice when generating search results
Comment 8 Carlos Garcia Campos 2012-08-31 12:41:21 UTC
Created attachment 223052 [details] [review]
yelp-document: Use xref: uris for search results
Comment 9 Carlos Garcia Campos 2012-08-31 12:42:25 UTC
Created attachment 223053 [details] [review]
yelp-uri: Use 'index' as default page for help: uris when resolving a xref
Comment 10 Carlos Garcia Campos 2012-08-31 12:43:20 UTC
Created attachment 223055 [details] [review]
yelp-view: Don't free the passed GError in view_show_error_page()
Comment 11 Carlos Garcia Campos 2012-08-31 12:44:15 UTC
Created attachment 223056 [details] [review]
yelp-view: Use custom URI schemes to handle document URIs
Comment 12 Carlos Garcia Campos 2012-08-31 12:47:22 UTC
The last patch requires new webkit API that hasn't landed yet, see bug:

https://bugs.webkit.org/show_bug.cgi?id=95549
Comment 13 Carlos Garcia Campos 2012-09-07 08:25:43 UTC
WebKit patch landed already 
http://trac.webkit.org/changeset/127749
Comment 14 Shaun McCance 2012-11-21 03:24:24 UTC
Review of attachment 223047 [details] [review]:

::: libyelp/yelp-document.c
@@ +1122,3 @@
 
+    if (page_id != NULL && g_str_has_prefix (page_id, "search="))
+        return g_strdup ("text/html");

I prefer application/xhtml+xml for anything created by Yelp.
Comment 15 Shaun McCance 2012-11-21 03:31:44 UTC
Review of attachment 223045 [details] [review]:

This is kind of peculiar, because a YelpUri contains information about the specific page, and not just the document. I need to look at this more.
Comment 16 Shaun McCance 2012-11-21 03:34:37 UTC
Comment on attachment 223046 [details] [review]
yelp-mallard-document: Fix use of uninitialized variable

Attachment 223046 [details] pushed as 8b88fcd - yelp-mallard-document: Fix use of uninitialized variable
Comment 17 Shaun McCance 2012-11-21 03:39:03 UTC
Review of attachment 223048 [details] [review]:

The function name implies it would have the equivalent functionality as yelp_document_get_for_uri(), when in fact it only does lookups of existing documents, and doesn't create new documents. Maybe _lookup_document_uri()?
Comment 18 Shaun McCance 2012-11-21 15:13:24 UTC
Comment on attachment 223050 [details] [review]
yelp-view: Fix memory leaks

Attachment 223050 [details] pushed as 10af959 - yelp-view: Fix memory leaks
Comment 19 Shaun McCance 2012-11-21 15:19:31 UTC
Review of attachment 223052 [details] [review]:

What's the rationale for this?
Comment 20 Shaun McCance 2012-11-21 15:26:32 UTC
Attachment 223051 [details] pushed as 59091e4 - yelp-document: Mutex is unlocked twice when generating search results
Attachment 223055 [details] pushed as 22ce877 - yelp-view: Don't free the passed GError in view_show_error_page()
Comment 21 Carlos Garcia Campos 2012-11-21 17:19:22 UTC
(In reply to comment #19)
> Review of attachment 223052 [details] [review]:
> 
> What's the rationale for this?

IIRC those links are resolved by will-send-request, using the fake URI trick. When using custom uri schemes, the relative paths are resolved using the uri loaded, so using xfer uris they are not considered relative, and they are handled like all other xfer uris.
Comment 22 Carlos Garcia Campos 2012-11-22 12:35:24 UTC
(In reply to comment #14)
> Review of attachment 223047 [details] [review]:
> 
> ::: libyelp/yelp-document.c
> @@ +1122,3 @@
> 
> +    if (page_id != NULL && g_str_has_prefix (page_id, "search="))
> +        return g_strdup ("text/html");
> 
> I prefer application/xhtml+xml for anything created by Yelp.

It's probably a bug in WebKitGTK+ but using application/xhtml+xml makes the contents be rendered as plain text. I would need to look at it in more detail.
Comment 23 Carlos Garcia Campos 2012-11-22 13:19:26 UTC
Created attachment 229631 [details] [review]
yelp-document: Add yelp_document_lookup_document_uri()
Comment 24 Carlos Garcia Campos 2012-11-22 13:20:29 UTC
Created attachment 229632 [details] [review]
yelp-view: Use custom URI schemes to handle document URIs

Updated to use yelp_document_lookup_document_uri()
Comment 25 Jarek Czekalski 2014-04-30 12:18:34 UTC
Hi. At what stage are we in fixing this bug? Can I help? How?

For 2 weeks I've been diving into yelp and webkit to fix some accessibility bugs. Now they told me I should rather think about webkit2, so I would like to help in the switch.
Comment 26 Carlos Garcia Campos 2014-04-30 13:06:20 UTC
Current WebKit allows to implement the hack of fake uris that are resolved in send-request signal, using a WebProcess extension. I still think custom uri schemes is the right way to implement this, though.
Comment 27 Carlos Garcia Campos 2014-04-30 13:07:40 UTC
Btw, I don't mind to rebase my patches and make them work with current webkit if there's any interest.
Comment 28 Marcos Chavarria Teijeiro 2014-09-09 09:52:27 UTC
Hi. Sure, rebasing that patches could be really useful!

Appart from that, what patches does need work??

The MIME type patch continues having the same problem, the application/xhtml+xml is rendered as plain text in Yelp and if I create a simple cgi script i python that returns a web with that mimetype and open that web with GNOME Web the content is rendered as XML. So it's probably a WebKit bug...
Comment 29 Marcos Chavarria Teijeiro 2014-09-18 11:03:04 UTC
Hi! I have been checking what patches are needed now for implementing the WK2 port and what pathes need to be rebased.

 * Make document-uri property a YelpUri -> The patch needs rebase.
 * Return HTML mime type insted of NULL for search pages -> We continue having the WK bug so application/xhtml+xml MIME type is rendered as plain text.
 * Use a full file:// for the JS -> We don't need that anymore the JS code has been moved to the Yelp-XSL module.
 * Use xref: uris for search results -> The patch applies in master!
 * Use 'index' as default page for help: uris when resolving a xref -> The patch applies in master!
 * Add yelp_document_lookup_document_uri() -> The patch needs rebase
 * Use custom URI schemes to handle document URIs -> We can implement this feature using the new WK2 API for custom schemes so this patch is not needed anymore.

Carlos, could you rebase those two patches? :)

And could some Yelp developer gives us some thoughts about those patches??
Comment 30 Marcos Chavarria Teijeiro 2014-09-19 07:49:12 UTC
Created attachment 286587 [details] [review]
Rebase of the YelpUri patch

I have made the rebase! I thought it would be more difficult...
Comment 31 Marcos Chavarria Teijeiro 2014-09-19 07:50:34 UTC
Created attachment 286588 [details] [review]
Rebase of the yelp_document_lookup_document_uri patch
Comment 32 Marcos Chavarria Teijeiro 2014-10-31 12:09:45 UTC
Created attachment 289725 [details] [review]
"return html mimetype" patch rebase

Rebase and fix of "return html mimetype instead of null for searchs results" Carlos patch.
Comment 33 Marcos Chavarria Teijeiro 2014-10-31 12:16:51 UTC
Just to sum up, the patches I've use to make the WK2 port have been:

1)Rebase of the YelpUri patch
2)Rebase of the yelp_document_lookup_document_uri patch
3)yelp-document: Use xref: uris for search results
4)"return html mimetype" patch rebase
5)yelp-uri: Use 'index' as default page for help: uris when resolving a xref