Bug 80401 – [BlackBerry] Credential save and autofill implemetation

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

[Jonathan Dong]

Implement credential autofill feature for blackberry porting.

Including http authentication dialog save and autofill, as well as in-form authentication save and autofill.

[Jonathan Dong]

Created attachment 130343 [details]
Patch

[Antonio Gomes]

Comment on attachment 130343 [details]
Patch

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

I will like we are missing an abstraction layer here :( ... the layering violation and the 'friend' class declaration makes me kinda of sad.

> Source/WebCore/platform/network/blackberry/NetworkJob.cpp:841
> +            Credential inputCredential = static_cast<FrameLoaderClientBlackBerry*>(m_frame->loader()->client())->authenticationChallenge(newURL, protectionSpace);

That is a layering violation.

[Jonathan Dong]

Created attachment 132177 [details]
Patch

[Rob Buis]

Comment on attachment 132177 [details]
Patch

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

Looks good, see if you can fix before landing, and Leo or Charles can help cq+ or commit for you.

> Source/WebKit/blackberry/Api/WebPage.cpp:2024
> +    WebCore::Credential inputCredential(username, password, WebCore::CredentialPersistenceNone);

You dont need WebCore:: prefix in this file.

> Source/WebKit/blackberry/Api/WebPage_p.h:180
> +    virtual PageClientBlackBerry::SaveCredentialType notifyShouldSaveCredential(bool);

PageClientBlackBerry:: prefix might be not needed.

[Jonathan Dong]

Created attachment 132948 [details]
Patch

[Rob Buis]

Comment on attachment 132948 [details]
Patch

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

Looks good.

> Source/WebCore/ChangeLog:6
> +        Reviewed by Rob Buis.

You probably mean internally reviewed here. Since I am doing both the internal and external review this is fine, but better keep it blank (i.e. NOBODY).

[Charles Wei]

Comment on attachment 132948 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        As this class is our platform independent interface,

should be "platform specific interface".

> Source/WebCore/ChangeLog:12
> +        FrameLoaderClient cause which might be intrusion.

"cause which might be intrusion" ==>which is a platform independent interface.

[Leo Yang]

You also need to address comment #4

[Rob Buis]

(In reply to comment #8)
> You also need to address comment #4

Ah, good catch...
Cheers,

Rob.

[Jonathan Dong]

Created attachment 132956 [details]
Patch

Sorry for the carelessness and thanks for the help

[Rob Buis]

Comment on attachment 132956 [details]
Patch

Looks good.

[WebKit Review Bot]

Comment on attachment 132956 [details]
Patch

Rejecting attachment 132956 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12035387

[Rob Buis]

Comment on attachment 132956 [details]
Patch

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

> Source/WebCore/ChangeLog:6
> +        Reviewed by NOBODY (OPPS!).

Typo!

> Source/WebKit/blackberry/ChangeLog:6
> +        Reviewed by NOBODY (OPPS!).

Ditto.

[Jonathan Dong]

Created attachment 133036 [details]
Patch

[Rob Buis]

Comment on attachment 133036 [details]
Patch

Lets retry.

[WebKit Review Bot]

Comment on attachment 133036 [details]
Patch

Clearing flags on attachment: 133036

Committed r111546: <http://trac.webkit.org/changeset/111546>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.