Bug 66561

Summary: [WK2] Support password echo feature in WebKit2
Product: WebKit Reporter: Chang Shu <cshu>
Component: FormsAssignee: Chang Shu <cshu>
Status: RESOLVED INVALID    
Severity: Normal CC: ap, darin, laszlo.gombos
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 32509, 68924, 69143    
Bug Blocks:    
Attachments:
Description Flags
patch 1 none

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.