[Webkit-unassigned] [Bug 73638] [EFL] Implement 'registerProtocolHandler' function

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73638





--- Comment #27 from Raphael Kubo da Costa (rakuco) <rakuco at webkit.org>  2012-05-08 19:15:15 PST ---
(From update of attachment 140831)
View in context: https://bugs.webkit.org/attachment.cgi?id=140831&action=review

Something that's not related to a specific part of the patch: what kind of functions do you intend to add? Doesn't it make sense to add them all in this patch?

> Source/WebKit/efl/ChangeLog:20
> +        * ewk/ewk_custom_handlers.cpp: Added.

If the function name prefix in this file is "ewk_custom_handler" (singular), the file should be called "ewk_custom_handler.cpp" (singular).

> Source/WebKit/efl/ewk/ewk_custom_handlers.cpp:29
> +    evas_object_smart_callback_call(data->ewkView, "protocolhandler,register", data);

I'm a bit confused about the behavior here -- is the intention of the code at this point to simply notify API users that this has happened or is this actively part of the process of registering a new protocol handler? In case it's the latter, then it probably makes sense to add a virtual function to the Ewk_View_Smart_Class structure and call it instead of emitting a signal.

> Source/WebKit/efl/ewk/ewk_view.h:73
> + *  - "protocolhandler,register", Ewk_Custom_Handler_Data: add a handler url for the given protocol.

Nit: we generally use the present perfect form for the verbs in signals ("displayed", "set") instead of the infinitive. After all, this is something that has already happened.

> Source/WebKit/efl/ewk/ewk_view.h:317
> +    const char *target; /**< Reference to the protocols that will be handled.  (eg. mailto, irc) */

Isn't this a single protocol instead of protocols (plural)? Shouldn't the variable be just called "protocol"?

> Source/WebKit/efl/ewk/ewk_view.h:318
> +    const char *base_url; /**< Reference to the resolved url if the url is relative url. (eg. "https://www.currenthost.com/") */

If the URL isn't relative, is this 0 or an empty string?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.