Bug 73177

Summary: Add Custom Content Handler
Product: WebKit Reporter: Dongwoo Joshua Im (dwim) <dw.im>
Component: WebCore Misc.Assignee: Dongwoo Joshua Im (dwim) <dw.im>
Status: RESOLVED INVALID    
Severity: Normal CC: abarth, annevk, eoconnor, eric, gbillock, ian, japhet, jasonliuwebkit, jhawkins, jochen, laszlo.gombos, leo.yang, mike, peter, p.jacquemart, rakuco, rcombs, s.choi, sh919.park, syoichi, webkit.review.bot, zeno
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: All   
OS: Linux   
See Also: https://bugs.webkit.org/show_bug.cgi?id=92749
https://bugs.webkit.org/show_bug.cgi?id=92726
Bug Depends on:    
Bug Blocks: 40829, 73639    
Attachments:
Description Flags
Patch none

Description Dongwoo Joshua Im (dwim) 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.
Comment 1 Alexey Proskuryakov 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.
Comment 2 Dongwoo Joshua Im (dwim) 2011-12-02 04:05:38 PST
I will implement 'registerContenHandler' API in this bug, also.
Comment 3 Eric Seidel (no email) 2012-01-20 18:01:13 PST
*** Bug 44740 has been marked as a duplicate of this bug. ***
Comment 4 Dongwoo Joshua Im (dwim) 2012-06-07 01:36:58 PDT
Created attachment 146229 [details]
Patch

the very first patch.
Comment 5 Adam Barth 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.
Comment 6 Adam Barth 2012-06-07 18:40:03 PDT
gbillock might have some insight into how this relates to WebIntents.
Comment 7 Adam Barth 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) ?
Comment 8 Adam Barth 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.
Comment 9 Dongwoo Joshua Im (dwim) 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.
Comment 10 Adam Barth 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.
Comment 11 Dongwoo Joshua Im (dwim) 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.
Comment 12 Greg Billock 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?
Comment 13 Greg Billock 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.
Comment 14 Dongwoo Joshua Im (dwim) 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.
Comment 15 Gyuyoung Kim 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 ?
Comment 16 Dongwoo Joshua Im (dwim) 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. :)
Comment 17 Dongwoo Joshua Im (dwim) 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.
Comment 18 Dongwoo Joshua Im (dwim) 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?
Comment 19 Ian 'Hixie' Hickson 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.
Comment 20 Dongwoo Joshua Im (dwim) 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?
Comment 21 Dongwoo Joshua Im (dwim) 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? :)
Comment 22 Anders Carlsson 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.
Comment 23 Anne van Kesteren 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.