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

[bugzilla-daemon at webkit.org]

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





--- Comment #29 from Dongwoo Joshua Im (dwim) <dw.im at samsung.com>  2012-05-08 20:25:51 PST ---
(In reply to comment #27)
> (From update of attachment 140831 [details])
> 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?
> 

I leave a comment above.


> > 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).
> 

Because I want to add Content Handler in this file alse.

I will just make it as singular form, 
and I will change it as plural when I add Content Handler.


> > 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.
> 

Yes. notify to the application.
As I know, that's what WebKit supposed to do all about.

I simply explained about this at the comment above.


> > 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.
> 

Registering a handler will be happened after an application get this signal.
I think "register" is right form in this case.

But, if it is a coding style of the EFL port, I will changed this  "protocolhandler,registered"


> > 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"?
> 

1. Single protocol. I will remove one of them.

2. I use the name "target" because I want to use this attribute for both protocol and content.

  I will change the name of this attribute as protocol, 
  and I will re-change it when I add Content Handler.


> > 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?

This is the return value of Document::baseURL().
This has value whether the url is relative or not.

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