RESOLVED INVALID 73177
Add Custom Content Handler
https://bugs.webkit.org/show_bug.cgi?id=73177
Summary Add Custom Content Handler
Dongwoo Joshua Im (dwim)
Reported 2011-11-27 18:03:37 PST
Regarding the HTML5 specification, (http://dev.w3.org/html5/spec/Overview.html#custom-handlers) two APIs are added for Custom Content Handler. One is 'isContentHandlerRegistered' to query whether the specific URL is registered or not. The other is 'unregisterContentHandler' to remove the registered URL. I'm preparing the patch to add these APIs now, I'll upload in a few days. And, here is one more thing. As I know, 'registerContentHandler' API is rolled out because there is no port to support the API. Is there any way to add the API again? I will implement Custom Content Handler on EFL port, including these new APIs. Thanks.
Attachments
Patch (54.96 KB, patch)
2012-06-07 01:36 PDT, Dongwoo Joshua Im (dwim)
no flags
Alexey Proskuryakov
Comment 1 2011-11-28 10:35:00 PST
Since this is a new feature, please send an e-mail to webkit-dev to discuss, so that interested parties could comment on implementation strategy, as well as on whether we want this feature in WebKit.
Dongwoo Joshua Im (dwim)
Comment 2 2011-12-02 04:05:38 PST
I will implement 'registerContenHandler' API in this bug, also.
Eric Seidel (no email)
Comment 3 2012-01-20 18:01:13 PST
*** Bug 44740 has been marked as a duplicate of this bug. ***
Dongwoo Joshua Im (dwim)
Comment 4 2012-06-07 01:36:58 PDT
Created attachment 146229 [details] Patch the very first patch.
Adam Barth
Comment 5 2012-06-07 18:38:37 PDT
It looks like there was a thread on this topic in 2011. I wonder if it would be worth pinging that thread again. Some things have changed since then. For example, I'm not sure how this relates to WebIntents.
Adam Barth
Comment 6 2012-06-07 18:40:03 PDT
gbillock might have some insight into how this relates to WebIntents.
Adam Barth
Comment 7 2012-06-07 18:42:28 PDT
Comment on attachment 146229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146229&action=review > Source/WebCore/page/NavigatorRegisterContentHandler.cpp:39 > +static void initContentHandlerBlacklist() This blacklist is troublesome. For example, what if we invent a new image format (e.g., image/webp) ?
Adam Barth
Comment 8 2012-06-07 18:44:20 PDT
I think the main question here is whether we should implement this feature. The patch looks to be in reasonably good shape from an implementation perspective. The use of a content-type blacklist worries me though. I wonder if the design needs some more thought in that respect.
Dongwoo Joshua Im (dwim)
Comment 9 2012-06-07 18:55:04 PDT
(In reply to comment #7) > (From update of attachment 146229 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146229&action=review > > > Source/WebCore/page/NavigatorRegisterContentHandler.cpp:39 > > +static void initContentHandlerBlacklist() > > This blacklist is troublesome. For example, what if we invent a new image format (e.g., image/webp) ? As you can see, some of blacklist have been added compared with first time, by W3C . Whenever we we add a new mime type, we can add it into the blacklist, and inform that to W3C to update the spec. If you want, I can follow up those things.
Adam Barth
Comment 10 2012-06-07 19:06:36 PDT
Won't that break folks who are using that content type? For registerProtocolHandler, we use the web+ prefix to solve this problem.
Dongwoo Joshua Im (dwim)
Comment 11 2012-06-08 00:59:13 PDT
(In reply to comment #10) > Won't that break folks who are using that content type? For registerProtocolHandler, we use the web+ prefix to solve this problem. 1. I'm a little bit unclear what you are concerning. at comment #7, you are worried about new mime type. I thought you are worried that new mime type might go to the handler rather than run on the browser itself. If it is right, that could be maintained what I suggested at comment #9 - maintaining the blacklist. But now at comment #10, I think you are worrying that the new mime type might not go to the handler which want to handler that mime type. 2. For registerProtocolHandler, we use "web+" prefix just like whitelist. If a protocol start with "web+", we can add the handler even if it is not in the whitelist. We can use similar strategy here.
Greg Billock
Comment 12 2012-06-08 08:56:30 PDT
Comment on attachment 146229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146229&action=review > Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:133 > +FEATURE_DEFINES = $(ENABLE_3D_RENDERING) $(ENABLE_ACCELERATED_2D_CANVAS) $(ENABLE_ANIMATION_API) $(ENABLE_BLOB) $(ENABLE_CHANNEL_MESSAGING) $(ENABLE_CSS3_FLEXBOX) $(ENABLE_CSS_EXCLUSIONS) $(ENABLE_CSS_FILTERS) $(ENABLE_CSS_IMAGE_RESOLUTION) $(ENABLE_CSS_REGIONS) $(ENABLE_CSS_SHADERS) $(ENABLE_CSS_VARIABLES) $(ENABLE_CUSTOM_CONTENT_HANDLER) $(ENABLE_CUSTOM_SCHEME_HANDLER) $(ENABLE_DASHBOARD_SUPPORT) $(ENABLE_DATALIST) $(ENABLE_DATA_TRANSFER_ITEMS) $(ENABLE_DETAILS) $(ENABLE_DEVICE_ORIENTATION) $(ENABLE_DIRECTORY_UPLOAD) $(ENABLE_FILE_SYSTEM) $(ENABLE_FILTERS) $(ENABLE_FONT_BOOSTING) $(ENABLE_FULLSCREEN_API) $(ENABLE_GAMEPAD) $(ENABLE_GEOLOCATION) $(ENABLE_HIGH_DPI_CANVAS) $(ENABLE_ICONDATABASE) $(ENABLE_IFRAME_SEAMLESS) $(ENABLE_INDEXED_DATABASE) $(ENABLE_INPUT_TYPE_COLOR) $(ENABLE_INPUT_SPEECH) $(ENABLE_INPUT_TYPE_DATE) $(ENABLE_INPUT_TYPE_DATETIME) $(ENABLE_INPUT_TYPE_DATETIMELOCAL) $(ENABLE_INPUT_TYPE_MONTH) $(ENABLE_INPUT_TYPE_TIME) $(ENABLE_INPUT_TYPE_WEEK) $(ENABLE_JAVASCRIPT_DEBUGGER) $(ENABLE_LEGACY_CSS_VENDOR_PREFIXES) $(ENABLE_LEGACY_NOTIFICATIONS) $(ENABLE_LINK_PREFETCH) $(ENABLE_LINK_PRERENDER) $(ENABLE_MATHML) $(ENABLE_MEDIA_SOURCE) $(ENABLE_MEDIA_STATISTICS) $(ENABLE_METER_TAG) $(ENABLE_MICRODATA) $(ENABLE_MUTATION_OBSERVERS) $(ENABLE_NOTIFICATIONS) $(ENABLE_PAGE_VISIBILITY_API) $(ENABLE_PROGRESS_TAG) $(ENABLE_QUOTA) $(ENABLE_REGISTER_PROTOCOL_HANDLER) $(ENABLE_REQUEST_ANIMATION_FRAME) $(ENABLE_SCRIPTED_SPEECH) $(ENABLE_SHADOW_DOM) $(ENABLE_SHARED_WORKERS) $(ENABLE_SQL_DATABASE) $(ENABLE_STYLE_SCOPED) $(ENABLE_SVG) $(ENABLE_SVG_DOM_OBJC_BINDINGS) $(ENABLE_SVG_FONTS) $(ENABLE_TEXT_NOTIFICATIONS_ONLY) $(ENABLE_TOUCH_ICON_LOADING) $(ENABLE_VIDEO) $(ENABLE_VIDEO_TRACK) $(ENABLE_WEBGL) $(ENABLE_WEB_AUDIO) $(ENABLE_WEB_SOCKETS) $(ENABLE_WEB_TIMING) $(ENABLE_WORKERS) $(ENABLE_XSLT); The registerProtocolHandler flag is "ENABLE_REGISTER_PROTOCOL_HANDLER" so I'd name this one "ENABLE_REGISTER_CONTENT_HANDLER". > Source/WebCore/page/NavigatorRegisterContentHandler.cpp:123 > + if (!verifyContentHandlerMimeType(mimeType, ec)) How about just calling iscontentBlacklisted directly here?
Greg Billock
Comment 13 2012-06-08 09:02:00 PDT
(In reply to comment #6) > gbillock might have some insight into how this relates to WebIntents. On the HTML list, we're considering basically seeing registerProtocolHandler, web intents, and registerContentHandler as APIs of the same feature. I don't think we'll end up getting rid of the RPH/RCH method signatures, though, so this is valuable. If that direction ends up being the one we take, we may want to end up merging all that code into one module, and having the APIs delegate to a single client signature. One caveat is that isContentHandlerRegistered has privacy implications that are worth considering specially.
Dongwoo Joshua Im (dwim)
Comment 14 2012-06-08 18:45:34 PDT
(In reply to comment #12) > (From update of attachment 146229 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146229&action=review > > > Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:133 > > > The registerProtocolHandler flag is "ENABLE_REGISTER_PROTOCOL_HANDLER" so I'd name this one "ENABLE_REGISTER_CONTENT_HANDLER". ENABLE_SCHEME_HANDLER flag is also there. I think ENABLE_SCHEME_HANDLER and ENABLE_CONTENT_HANDLER are better. I filed that bug already, https://bugs.webkit.org/show_bug.cgi?id=88614. > > > Source/WebCore/page/NavigatorRegisterContentHandler.cpp:123 > > + if (!verifyContentHandlerMimeType(mimeType, ec)) > > How about just calling iscontentBlacklisted directly here? That could be. I will try that.
Gyuyoung Kim
Comment 15 2012-06-09 05:08:05 PDT
Comment on attachment 146229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146229&action=review As Greg mentioned in commit #13, it looks this feature is able to be moved to Modules. > Source/WebCore/page/NavigatorRegisterContentHandler.cpp:64 > + // The specification requires that it is a SYNTAX_ERR if the "%s" token is Don't you add "FIXME:" to here ?
Dongwoo Joshua Im (dwim)
Comment 16 2012-06-10 17:23:04 PDT
(In reply to comment #15) > (From update of attachment 146229 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146229&action=review > > As Greg mentioned in commit #13, it looks this feature is able to be moved to Modules. As Greg said, the three features - registerProtocolHandler, web intents, and registerContentHandler - will be merged as one modlue. I think we can move this after those are merged as one module. Or, I can move registerProtocolHandler and registerContentHandler into Modules/customhandler now. Which one is better? > > > Source/WebCore/page/NavigatorRegisterContentHandler.cpp:64 > > + // The specification requires that it is a SYNTAX_ERR if the "%s" token is > > Don't you add "FIXME:" to here ? I don't think FIXME is needed here. :)
Dongwoo Joshua Im (dwim)
Comment 17 2012-06-10 21:45:02 PDT
(In reply to comment #14) > (In reply to comment #12) > > (From update of attachment 146229 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=146229&action=review > > > > > Source/WebCore/page/NavigatorRegisterContentHandler.cpp:123 > > > + if (!verifyContentHandlerMimeType(mimeType, ec)) > > > > How about just calling iscontentBlacklisted directly here? > > That could be. > I will try that. I think we can leave this function, so that this function can handle more verification by calling other functions in itself.
Dongwoo Joshua Im (dwim)
Comment 18 2012-06-11 22:30:24 PDT
Dear Ian Hickson, We have had discussion about the Custom Content Handler which is defined in HTML5 spec. We are concerning that this feature is still valuable to implement in WebKit or not. Could you please give your opinion about this?
Ian 'Hixie' Hickson
Comment 19 2012-06-12 13:38:19 PDT
I expect this and Web Intents will merge, but I don't expect it will do so in a way that breaks backwards-compatibility with this feature, so it should be fine to implement. Might want to check with the Web Intents guys though; I thought they were ready for the merge but then they started pushing their spec through the W3C system so I don't know what their plan is at the moment.
Dongwoo Joshua Im (dwim)
Comment 20 2012-06-12 17:53:47 PDT
(In reply to comment #19) > I expect this and Web Intents will merge, but I don't expect it will do so in a way that breaks backwards-compatibility with this feature, so it should be fine to implement. Might want to check with the Web Intents guys though; I thought they were ready for the merge but then they started pushing their spec through the W3C system so I don't know what their plan is at the moment. Dear Greg Billock, Could you please give your opinion about this? ;) What kind of changes will be added in this feature? Could you share your plan? And, still you think it is valuable to be implemented in WebKit as is?
Dongwoo Joshua Im (dwim)
Comment 21 2012-10-15 00:32:51 PDT
Custom Content Handler is still in the very current HTML5 spec, which is release on 11th October 2012. Still, do you guys think this isn't valuable to implement in WebKit? Any other news or opinion about this? :)
Anders Carlsson
Comment 22 2014-02-05 10:55:38 PST
Comment on attachment 146229 [details] Patch Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.
Anne van Kesteren
Comment 23 2023-04-03 04:48:30 PDT
If there is an initiative to add this API again it would benefit from a clean approach so let's close this.
Note You need to log in before you can comment on or make changes to this bug.