Bug 66561 - [WK2] Support password echo feature in WebKit2
Summary: [WK2] Support password echo feature in WebKit2
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Chang Shu
URL:
Keywords:
Depends on: 32509 68924 69143
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-19 07:58 PDT by Chang Shu
Modified: 2011-10-07 12:04 PDT (History)
3 users (show)

See Also:


Attachments
patch 1 (2.79 KB, patch)
2011-10-05 14:21 PDT, Chang Shu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chang Shu 2011-08-19 07:58:14 PDT
Some additional work needs to do in WebKit2 for this feature.
Comment 1 Chang Shu 2011-10-05 14:21:11 PDT
Created attachment 109861 [details]
patch 1
Comment 2 Darin Adler 2011-10-05 15:32:14 PDT
Comment on attachment 109861 [details]
patch 1

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1707
> +#if ENABLE(PASSWORD_ECHO)
> +    settings->setPasswordEchoEnabled(true);
> +    settings->setPasswordEchoDurationInSeconds(1);
> +#endif

Password echo turned on unconditionally if it’s compiled at all? That just doesn’t seem right. If that was the policy then I’d expect to see that policy in WebCore.
Comment 3 Chang Shu 2011-10-05 16:02:05 PDT
(In reply to comment #2)
> (From update of attachment 109861 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109861&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1707
> > +#if ENABLE(PASSWORD_ECHO)
> > +    settings->setPasswordEchoEnabled(true);
> > +    settings->setPasswordEchoDurationInSeconds(1);
> > +#endif
> 
> Password echo turned on unconditionally if it’s compiled at all? That just doesn’t seem right. If that was the policy then I’d expect to see that policy in WebCore.

By default, PASSWORD_ECHO is not enabled. If the feature is chosen to turn on, a build time flag is required. An alternative way is to enable it in runtime. Alexey and I have discussed on whether the application(such as MiniBrowser) should take care of this (during runtime) but we chose to control this in WebKit. WebCore seems a place to provide this option in the settings.
Comment 4 Alexey Proskuryakov 2011-10-05 16:15:22 PDT
I remember discussing that every application on a platform should not be required to do something to follow default platform behavior for password echo. Details of proposed solution are a bit vague - this was a while ago.
Comment 5 Darin Adler 2011-10-05 16:31:26 PDT
(In reply to comment #4)
> I remember discussing that every application on a platform should not be required to do something to follow default platform behavior for password echo. Details of proposed solution are a bit vague - this was a while ago.

It seems strange to have something defaulting off at the WebCore level then turned on unconditionally at the WebKit level.
Comment 6 Chang Shu 2011-10-05 18:21:49 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > I remember discussing that every application on a platform should not be required to do something to follow default platform behavior for password echo. Details of proposed solution are a bit vague - this was a while ago.
> 
> It seems strange to have something defaulting off at the WebCore level then turned on unconditionally at the WebKit level.

The "condition" is the build flag. In fact, symbian has already defined ENABLE_PASSWORD_ECHO=1 in WebCore/config.h. And other platforms are disabled by default. In anther way, we can set different default value using #if PLATFORM() flag in WebCore. Maybe we should do this.

Btw, just got the sad news.
Comment 7 Chang Shu 2011-10-07 12:04:57 PDT
We don't need this any more. The compile time is removed in bug 69647 so we don't need any special treatment in WK2 code. The unskipped tests are included there, too.