Bug 85577 - [BlackBerry] Autofill feature implementation for BlackBerry porting
Summary: [BlackBerry] Autofill feature implementation for BlackBerry porting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Dong
URL:
Keywords:
Depends on: 85576
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 21:19 PDT by Jonathan Dong
Modified: 2012-06-11 17:32 PDT (History)
7 users (show)

See Also:


Attachments
Patch (11.54 KB, patch)
2012-05-09 05:16 PDT, Jonathan Dong
no flags Details | Formatted Diff | Diff
Patch (10.73 KB, patch)
2012-05-09 19:10 PDT, Jonathan Dong
no flags Details | Formatted Diff | Diff
Patch (2.57 KB, patch)
2012-06-11 02:17 PDT, Jonathan Dong
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Dong 2012-05-03 21:19:06 PDT
RIM PR: 136405

The form auto fill feature by my current implementation is pretty much like the behavior of desktop browsers, when user submits a form, the name attribute and the value of text input fields(exclude password) will be stored in the autofill database as a name-value pair. Next time when the user fills any form on any site, the browser will search the db to find out if there's any matched name-value pair to fill the current editing input text field. We will find all the values which has the same name attribute and starts with the character that user has input, and pops up a context dialog to notify user those candidates used by him. If user chooses one from the context dialog, the value will fill the current input text field.

his PR is about the implementation of autofill feature in webkit side.
Comment 1 Jonathan Dong 2012-05-09 05:16:31 PDT
Created attachment 140923 [details]
Patch
Comment 2 Rob Buis 2012-05-09 09:06:32 PDT
Comment on attachment 140923 [details]
Patch

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

Looks good, please fix the issues before landing.

> Source/WebKit/blackberry/Api/WebPage.cpp:2136
> +        textItems.push_back(candidates[i].utf8().data());

It would be great if we would not have to do the WTF::Vector to vector conversion but I guess that might be hard...

> Source/WebKit/blackberry/Api/WebPage_p.h:193
> +    void notifyPopupAutofillDialog(const WTF::Vector<WTF::String>&, const WebCore::IntRect&);

I don't think you need the WTF:: prefixes here.

> Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.h:114
> +

No need for this empty line.

> Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:724
> +credentialManager().saveCredentialIfConfirmed(m_webPagePrivate, CredentialTransformData(prpFormState->form()));

Indent properly.
Comment 3 Jonathan Dong 2012-05-09 18:34:12 PDT
Comment on attachment 140923 [details]
Patch

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

>> Source/WebKit/blackberry/Api/WebPage.cpp:2136
>> +        textItems.push_back(candidates[i].utf8().data());
> 
> It would be great if we would not have to do the WTF::Vector to vector conversion but I guess that might be hard...

Thanks Rob, well this vector is generated from autofill database querying, those codes are in WebCore/platform/network/blackberry, which suppose we should not use std::vector there. That means we need to convert the WTF::Vector to std::vector somewhere before we send them out of webkit.
Fortunately the overhead is not heavy by my test for now. To solve this issue I think we can move AutofillBackingStore.[cpp|h] into WebKit/blackberry/WebCoreSupport and replace the WTF::Vector with std::vector in the future patch.
Comment 4 Jonathan Dong 2012-05-09 19:10:36 PDT
Created attachment 141072 [details]
Patch
Comment 5 Rob Buis 2012-05-09 19:46:54 PDT
Comment on attachment 141072 [details]
Patch

LGTM.
Comment 6 WebKit Review Bot 2012-05-09 20:57:45 PDT
Comment on attachment 141072 [details]
Patch

Clearing flags on attachment: 141072

Committed r116603: <http://trac.webkit.org/changeset/116603>
Comment 7 WebKit Review Bot 2012-05-09 20:57:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Jonathan Dong 2012-06-11 02:16:59 PDT
Reopening to attach new patch.
Comment 9 Jonathan Dong 2012-06-11 02:17:15 PDT
Created attachment 146815 [details]
Patch
Comment 10 Rob Buis 2012-06-11 07:02:42 PDT
Comment on attachment 146815 [details]
Patch

Makes sense.
Comment 11 WebKit Review Bot 2012-06-11 17:32:25 PDT
Comment on attachment 146815 [details]
Patch

Clearing flags on attachment: 146815

Committed r120022: <http://trac.webkit.org/changeset/120022>
Comment 12 WebKit Review Bot 2012-06-11 17:32:29 PDT
All reviewed patches have been landed.  Closing bug.