Bug 47256

Summary: Adding C api for <select> popups
Product: WebKit Reporter: Luiz Agostini <luiz>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: andersca, aroben, beidson, cgarcia, darin, hausmann, kenneth, kimmo.t.kinnunen, koivisto, laszlo.gombos, mjs, sam, yael
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 43428    

Description Luiz Agostini 2010-10-06 04:31:10 PDT
Here goes a first proposal for the C api to be used for combobox handling in webkit2. Comments are very welcome.


typedef const struct OpaqueWKCombobox* WKComboboxRef;

typedef void (*WKPageShowComboboxPopupCallback)(WKPageRef page, WKComboboxRef combobox, const void *clientInfo);
typedef void (*WKPageHideComboboxPopupCallback)(WKPageRef page, WKComboboxRef combobox, const void *clientInfo);

struct WKPageComboboxClient {
    int                                                                 version;
    const void *                                                        clientInfo;
    WKPageShowComboboxPopupCallback                                     show;
    WKPageShowComboboxPopupCallback                                     hide;
};
typedef struct WKPageComboboxClient WKPageComboboxClient;

WK_EXPORT void WKComboboxSelectItem(WKComboboxRef combobox, int index, bool ctrl, bool shift);
WK_EXPORT void WKComboboxDidHide(WKComboboxRef combobox);

WK_EXPORT int WKComboboxGetItemCount(WKComboboxRef combobox);
WK_EXPORT bool WKComboboxGetIsMultiple(WKComboboxRef combobox);

enum {
    kWKComboboxItemTypeOption = 0,
    kWKComboboxItemTypeGroup = 1,
    kWKComboboxItemTypeSeparator = 2
};
typedef uint32_t WKComboboxItemType;

WK_EXPORT WKComboboxItemType WKComboboxGetItemItype(WKComboboxRef combobox, int index);
WK_EXPORT WKStringRef WKComboboxCopyItemText(WKComboboxRef combobox, int index);
WK_EXPORT WKStringRef WKComboboxCopyItemTooltip(WKComboboxRef combobox, int index);
WK_EXPORT bool WKComboboxGetItemIsEnabled(WKComboboxRef combobox, int index);
WK_EXPORT bool WKComboboxGetItemIsSelected(WKComboboxRef combobox, int index);
Comment 1 Kenneth Rohde Christiansen 2010-10-06 04:42:52 PDT
(In reply to comment #0)
> Here goes a first proposal for the C api to be used for combobox handling in webkit2. Comments are very welcome.
> 
> 
> typedef const struct OpaqueWKCombobox* WKComboboxRef;

What about WKSelectPopupRef. It is for dealing with select input and it is supposed to render on top of the web contents.

> WK_EXPORT void WKComboboxDidHide(WKComboboxRef combobox);

Is this like a "inform did hide"? I think we need a better name for that one.

> 
> WK_EXPORT int WKComboboxGetItemCount(WKComboboxRef combobox);
> WK_EXPORT bool WKComboboxGetIsMultiple(WKComboboxRef combobox);

SupportsMultiplySelections? I do not think it is clear what "IsMultiple" means.
Comment 2 Kenneth Rohde Christiansen 2010-10-06 04:44:03 PDT
> SupportsMultiplySelections? I do not think it is clear what "IsMultiple" means.

AllowMultiplySelections is probably better.
Comment 3 Adam Roben (:aroben) 2010-10-06 04:51:58 PDT
I assume by "combobox" you mean <select> elements with a size attribute less than 2?

I think it would be useful to talk about use-cases for the API in addition to the API itself. One should inform the other. For instance, it isn't clear to me why this needs to be handled in the application at all; couldn't WebKit handle this itself in the UI process?
Comment 4 Kenneth Rohde Christiansen 2010-10-06 04:57:39 PDT
(In reply to comment #3)
> I assume by "combobox" you mean <select> elements with a size attribute less than 2?
> 
> I think it would be useful to talk about use-cases for the API in addition to the API itself. One should inform the other. For instance, it isn't clear to me why this needs to be handled in the application at all; couldn't WebKit handle this itself in the UI process?

It is not just for size attributes less than two. Like on the iphone we supports using this for single and multiple selections.

For Qt we have a qtwebkitplatform plugin, where platforms such as MeeGo, Symbian etc can decide how this should be handled in a platform way. If that plugin is not installed we will deal with it using normal desktop like QWidgets, which do not work nicely for phones, TV's etc.
Comment 5 Luiz Agostini 2010-10-06 05:27:01 PDT
(In reply to comment #3)
> I assume by "combobox" you mean <select> elements with a size attribute less than 2?

Actually I mean all <select> elements, with any size or multiple attributes. If ENABLE_NO_LISTBOX_RENDERING=1 is defined at compile time then all <select> will be rendered as menu lists.

WKSelectPopupRef is probably a better name then WKComboboxRef.

> 
> I think it would be useful to talk about use-cases for the API in addition to the API itself. One should inform the other. For instance, it isn't clear to me why this needs to be handled in the application at all; couldn't WebKit handle this itself in the UI process?

As Kenneth explained our users may provide menu lists popup customizations using a plugin mechanism.
Comment 6 Luiz Agostini 2010-10-06 05:39:45 PDT
(In reply to comment #1)
> (In reply to comment #0)
> > Here goes a first proposal for the C api to be used for combobox handling in webkit2. Comments are very welcome.
> > 
> > 
> > typedef const struct OpaqueWKCombobox* WKComboboxRef;
> 
> What about WKSelectPopupRef. It is for dealing with select input and it is supposed to render on top of the web contents.

I agree.

> 
> > WK_EXPORT void WKComboboxDidHide(WKComboboxRef combobox);
> 
> Is this like a "inform did hide"? I think we need a better name for that one.

I just dei not find a better option. Any suggestion?

> 
> > 
> > WK_EXPORT int WKComboboxGetItemCount(WKComboboxRef combobox);
> > WK_EXPORT bool WKComboboxGetIsMultiple(WKComboboxRef combobox);
> 
> SupportsMultiplySelections? I do not think it is clear what "IsMultiple" means.

ok.

Hera goes a new version of the proposal.

typedef const struct OpaqueWKSelectPopup* WKSelectPopupRef;

typedef void (*WKPageShowSelectPopupPopupCallback)(WKPageRef page, WKSelectPopupRef select, const void *clientInfo);
typedef void (*WKPageHideSelectPopupPopupCallback)(WKPageRef page, WKSelectPopupRef select, const void *clientInfo);

struct WKPageSelectPopupClient {
    int                                                                 version;
    const void *                                                        clientInfo;
    WKPageShowSelectPopupPopupCallback                                  show;
    WKPageShowSelectPopupPopupCallback                                  hide;
};
typedef struct WKPageSelectPopupClient WKPageSelectPopupClient;

WK_EXPORT void WKSelectPopupSelectItem(WKSelectPopupRef select, int index, bool ctrl, bool shift);
WK_EXPORT void WKSelectPopupDidHide(WKSelectPopupRef select);

WK_EXPORT int WKSelectPopupGetItemCount(WKSelectPopupRef select);
WK_EXPORT bool WKSelectPopupGetAllowMultiplySelections(WKSelectPopupRef select);

enum {
    kWKSelectPopupItemTypeOption = 0,
    kWKSelectPopupItemTypeGroup = 1,
    kWKSelectPopupItemTypeSeparator = 2
};
typedef uint32_t WKSelectPopupItemType;

WK_EXPORT WKSelectPopupItemType WKSelectPopupGetItemItype(WKSelectPopupRef select, int index);
WK_EXPORT WKStringRef WKSelectPopupCopyItemText(WKSelectPopupRef select, int index);
WK_EXPORT WKStringRef WKSelectPopupCopyItemTooltip(WKSelectPopupRef select, int index);
WK_EXPORT bool WKSelectPopupGetItemIsEnabled(WKSelectPopupRef select, int index);
WK_EXPORT bool WKSelectPopupGetItemIsSelected(WKSelectPopupRef select, int index);
WK_EXPORT bool WKSelectPopupGetItemIsInGroup(WKSelectPopupRef select, int index);

This last function was not in previous proposal. It would be used to know if a certain item is part of a previous group. Without it it is not possible to identify the end of a group.
Comment 7 Luiz Agostini 2010-10-06 05:43:15 PDT
This proposal just cover a small part of the items in PopupMenuClient. Style information for example is not considered. All missing information may be added later by adding new getters.
Comment 8 Kenneth Rohde Christiansen 2010-10-06 05:51:13 PDT
WK_EXPORT bool WKSelectPopupGetItemIsInGroup(WKSelectPopupRef select, int index);

How does this work? the index is the group index or the index of the item?

--
Regarding:
WK_EXPORT void WKSelectPopupDidHide(WKComboboxRef combobox);

What about 

WK_EXPORT void WKSelectPopupInformDidHide(WKComboboxRef combobox);
Comment 9 Kenneth Rohde Christiansen 2010-10-06 05:53:26 PDT
struct WKPageSelectPopupClient {

Maybe this should be called WKPageSelectPopupUIClient
Comment 10 Luiz Agostini 2010-10-06 06:20:52 PDT
(In reply to comment #8)
> WK_EXPORT bool WKSelectPopupGetItemIsInGroup(WKSelectPopupRef select, int index);
> 
> How does this work? the index is the group index or the index of the item?

index is the index of the item. 

for example:

<select>
    <optgroup label="group">
        <option value="red">red</option>
        <option value="green">green</option>
    </optgroup>
    <option value="blue">blue</option>
</select>

WKSelectPopupGetItemIsInGroup would return true for indexes 1 and 2 and return false for index 3 indicating that blue is not a part of the previous group. This should be enough since just one level of groups is allowed. 

I could replace it by a method that returns the item level or indentation. Or by a method that returns the index of the group to which the item belongs to or -1 if it does not belong to any group.

Suggestions?

> 
> --
> Regarding:
> WK_EXPORT void WKSelectPopupDidHide(WKComboboxRef combobox);
> 
> What about 
> 
> WK_EXPORT void WKSelectPopupInformDidHide(WKComboboxRef combobox);

It is better then just did hide for sure.
Comment 11 Darin Adler 2010-10-06 10:04:04 PDT
I’m not sure this is the right direction. We should not land anything here until we hear from Sam Weinig, Anders Carlsson, and Maciej Stachowiak. Do we really need API for this?

The term “combo box” is a Windows-specific one that I don’t see at all in HTML specifications and doesn’t make sense to me for a cross-platform API. I’d prefer that we not include this phrase in our function names.
Comment 12 Kenneth Rohde Christiansen 2010-10-06 10:07:11 PDT
(In reply to comment #11)
> I’m not sure this is the right direction. We should not land anything here until we hear from Sam Weinig, Anders Carlsson, and Maciej Stachowiak. Do we really need API for this?
> 
> The term “combo box” is a Windows-specific one that I don’t see at all in HTML specifications and doesn’t make sense to me for a cross-platform API. I’d prefer that we not include this phrase in our function names.

I agree with that. We need something like this for Qt, but it can be a SPI, but it would be nice if we can define how to do SPI's for WebKit2.
Comment 13 Sam Weinig 2010-10-06 22:01:45 PDT
(In reply to comment #11)
> I’m not sure this is the right direction. We should not land anything here until we hear from Sam Weinig, Anders Carlsson, and Maciej Stachowiak. Do we really need API for this?
> 
> The term “combo box” is a Windows-specific one that I don’t see at all in HTML specifications and doesn’t make sense to me for a cross-platform API. I’d prefer that we not include this phrase in our function names.

This is definitely seems like the wrong direction to go.  Much like the reset of the UI elements in a web page, the <select> element should be handled by the engine, not the embedder.  Can you explain a bit more why you think this should not just be implemented in WebKit itself?
Comment 14 Luiz Agostini 2010-10-07 01:55:06 PDT
(In reply to comment #13)
> (In reply to comment #11)
> > I’m not sure this is the right direction. We should not land anything here until we hear from Sam Weinig, Anders Carlsson, and Maciej Stachowiak. Do we really need API for this?
> > 
> > The term “combo box” is a Windows-specific one that I don’t see at all in HTML specifications and doesn’t make sense to me for a cross-platform API. I’d prefer that we not include this phrase in our function names.
> 
> This is definitely seems like the wrong direction to go.  Much like the reset of the UI elements in a web page, the <select> element should be handled by the engine, not the embedder.  Can you explain a bit more why you think this should not just be implemented in WebKit itself?

This is because users of QtWebKit may provide customizations for the <select> popups.

In mobile platforms, for example, we want all our <select> elements to be rendered as menu lists, even multiple selection ones and the ones which have size > 1. But for it to work we need to provide a menu list popup that is big enough to provide a good user experience and that is capable of handling multiple selections. Something similar goes on in iPhone.

Today we have a plugin mechanism that can be used to provide menu list popup customizations. If a plugin is provided then the popup handling is delegated to the plugin. If no plugin is provided then we use the standard popup implementation. Different platforms may decide how to implement their own popups in their own way. We already have some different implementations for meego, symbian, etc.

The objective of this api is to provide a way to do the same in WebKit2.
Comment 15 Kenneth Rohde Christiansen 2010-10-07 07:37:56 PDT
I don't think we need a public C api for this. We can just hook it up to our current platform plugin architecture from the UIProcess side.
Comment 16 Luiz Agostini 2010-10-07 09:14:53 PDT
(In reply to comment #15)
> I don't think we need a public C api for this. We can just hook it up to our current platform plugin architecture from the UIProcess side.

But is not this a valid use case of the <select> popup been handled outside the engine?
Comment 17 Kenneth Rohde Christiansen 2011-04-12 12:37:43 PDT
Laszlo, Yael, for your information, I have modied patches for this on the qtwebkit2 gitorious branch.
Comment 18 Yael 2011-04-12 17:25:38 PDT
(In reply to comment #17)
> Laszlo, Yael, for your information, I have modied patches for this on the qtwebkit2 gitorious branch.

Thanks for letting us know.
Do you have the functionality or an actual plugin in that branch?
Comment 19 Kenneth Rohde Christiansen 2011-04-13 01:08:37 PDT
(In reply to comment #18)
> (In reply to comment #17)
> > Laszlo, Yael, for your information, I have modied patches for this on the qtwebkit2 gitorious branch.
> 
> Thanks for letting us know.
> Do you have the functionality or an actual plugin in that branch?

We are not using a plugin on the branch, so it is possible for us to better integrate it with virtual keyboard handling.

You should be able to find the patches quite easily. If not I can probably help you :-)