Bug 89749 - [EFL][WK2] Add API to inspect a Web Intent
Summary: [EFL][WK2] Add API to inspect a Web Intent
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on: 89275
Blocks: 90064
  Show dependency treegraph
 
Reported: 2012-06-22 01:43 PDT by Chris Dumez
Modified: 2012-07-01 19:13 PDT (History)
5 users (show)

See Also:


Attachments
Patch (19.53 KB, patch)
2012-06-22 02:20 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (19.67 KB, patch)
2012-06-26 23:19 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (19.62 KB, patch)
2012-06-28 00:36 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (19.82 KB, patch)
2012-06-29 01:07 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-06-22 01:43:35 PDT
The WK2 C API to inspect a Web Intent recently landed in Bug 89275. We now need to provide an Ewk wrapper in WK2 EFL port.
Comment 1 Chris Dumez 2012-06-22 02:20:52 PDT
Created attachment 148982 [details]
Patch
Comment 2 Chris Dumez 2012-06-26 05:47:37 PDT
Informal review anyone?
Comment 3 Gyuyoung Kim 2012-06-26 19:40:52 PDT
Comment on attachment 148982 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148982&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_intent.h:25
> +

It is good to write file description.

> Source/WebKit2/UIProcess/API/efl/ewk_intent.h:115
> +EAPI char *ewk_intent_extra_get(const Ewk_Intent *intent, const char *key);

I wonder why this APIs doesn't use const char* as other APIs. Is there any reason ?
Comment 4 Chris Dumez 2012-06-26 23:14:12 PDT
Comment on attachment 148982 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148982&action=review

>> Source/WebKit2/UIProcess/API/efl/ewk_intent.h:115
>> +EAPI char *ewk_intent_extra_get(const Ewk_Intent *intent, const char *key);
> 
> I wonder why this APIs doesn't use const char* as other APIs. Is there any reason ?

This is because using stringsharing is not easy in this case, since we have a hash table.
Comment 5 Chris Dumez 2012-06-26 23:19:13 PDT
Created attachment 149688 [details]
Patch

Add missing file description for ewk_intent.h.
Comment 6 Gyuyoung Kim 2012-06-27 18:01:06 PDT
Comment on attachment 149688 [details]
Patch

Looks good to me.
Comment 7 Chris Dumez 2012-06-28 00:36:21 PDT
Created attachment 149896 [details]
Patch

- No longer ignores intent requests coming from other frames than the main one.
- Fix signal name. It is now "intent,request,new" instead of "title,changed" (bad copy / paste).
- Rename ewk_view_intent_new() to ewk_view_intent_request_new() for clarity.
Comment 8 Chris Dumez 2012-06-29 01:07:26 PDT
Created attachment 150110 [details]
Patch

Rebase on master.
Comment 9 Kenneth Rohde Christiansen 2012-06-29 02:08:26 PDT
Comment on attachment 150110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150110&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_intent.cpp:46
> +struct _Ewk_Intent {
> +    unsigned int __ref; /**< the reference count of the object */

ieck manual ref counting :-(

> Source/WebKit2/UIProcess/API/efl/ewk_intent.cpp:149
> +char* ewk_intent_extra_get(const Ewk_Intent* intent, const char* key)

extra what? :-) additional_metadata ? Or is this part of the spec?
Comment 10 Chris Dumez 2012-06-29 02:28:46 PDT
Comment on attachment 150110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150110&action=review

>> Source/WebKit2/UIProcess/API/efl/ewk_intent.cpp:46
>> +    unsigned int __ref; /**< the reference count of the object */
> 
> ieck manual ref counting :-(

I copied the style used in other Ewk classes.

>> Source/WebKit2/UIProcess/API/efl/ewk_intent.cpp:149
>> +char* ewk_intent_extra_get(const Ewk_Intent* intent, const char* key)
> 
> extra what? :-) additional_metadata ? Or is this part of the spec?

It is part of the spec, yes: http://www.w3.org/TR/web-intents/#methods
Comment 11 WebKit Review Bot 2012-07-01 19:12:59 PDT
Comment on attachment 150110 [details]
Patch

Clearing flags on attachment: 150110

Committed r121649: <http://trac.webkit.org/changeset/121649>
Comment 12 WebKit Review Bot 2012-07-01 19:13:06 PDT
All reviewed patches have been landed.  Closing bug.