Bug 89777 – [BlackBerry] Use air controls if client doesn't support HTML control

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Crystal Zhang]

Currently HTML controls depend on client implementing CreateWindow event, for those clients that don't implement CreateWindow event, use old air controls so we don't break them.

[Crystal Zhang]

Created attachment 149095 [details]
patch

[Yong Li]

Comment on attachment 149095 [details]
patch

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

> Source/WebKit/blackberry/Api/WebPageClient.h:163
> +    virtual void openPopupList(bool multiple, int size, const ScopeArray<WebString>& labels, bool* enableds, const int* itemType, bool* selecteds) = 0;

"size" is a little bit vague here. Are "enableds" and "selecteds" outputs?

> Source/WebKit/blackberry/Api/WebPageClient.h:255
> -    virtual void createPopupWebView(const Platform::IntRect& webViewRect) = 0;
> +    virtual bool createPopupWebView(Platform::IntRect) = 0;

why remove const&?

> Source/WebKit/blackberry/ChangeLog:4
> +        [BlackBerry] Use air controls if client doesn't support HTML control
> +        https://bugs.webkit.org/show_bug.cgi?id=89777

air=>AIR?

[Crystal Zhang]

Comment on attachment 149095 [details]
patch

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

>> Source/WebKit/blackberry/Api/WebPageClient.h:163
>> +    virtual void openPopupList(bool multiple, int size, const ScopeArray<WebString>& labels, bool* enableds, const int* itemType, bool* selecteds) = 0;
> 
> "size" is a little bit vague here. Are "enableds" and "selecteds" outputs?

no, they are inputs. Actually I just put the old method back, I thought we don't need it any more so removed in previous patch.

>> Source/WebKit/blackberry/Api/WebPageClient.h:255
>> +    virtual bool createPopupWebView(Platform::IntRect) = 0;
> 
> why remove const&?

In our repo it's just Platform::IntRect, also in libwebview it's the same too. Just want to make it consistent, otherwise we need to change libwebview too.

>> Source/WebKit/blackberry/ChangeLog:4
>> +        https://bugs.webkit.org/show_bug.cgi?id=89777
> 
> air=>AIR?

yeah.

[Crystal Zhang]

Created attachment 149110 [details]
updated patch

[Mike Fenton]

Comment on attachment 149110 [details]
updated patch

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

> Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp:1178
> +    if (m_webPage->m_page->chrome()->client()->openPagePopup(selectClient, elementRectInRootView))

Why not make this if (!..) And then only have one return true?

[Crystal Zhang]

Created attachment 149292 [details]
updated patch

[WebKit Review Bot]

Comment on attachment 149292 [details]
updated patch

Clearing flags on attachment: 149292

Committed r121158: <http://trac.webkit.org/changeset/121158>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.