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
Dongwoo Joshua Im (dwim)
2011-11-27 18:03:37 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. I will implement 'registerContenHandler' API in this bug, also. *** Bug 44740 has been marked as a duplicate of this bug. *** Created attachment 146229 [details]
Patch
the very first patch.
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. gbillock might have some insight into how this relates to WebIntents. 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) ? 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. (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. Won't that break folks who are using that content type? For registerProtocolHandler, we use the web+ prefix to solve this problem. (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 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? (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. (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 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 ? (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. :) (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. 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? 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. (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? 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 on attachment 146229 [details]
Patch
Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.
If there is an initiative to add this API again it would benefit from a clean approach so let's close this. |