Secure input can be enabled permanently due to logic error

[Greg Scown]

Please refer to - [WKView _updateSecureInputState] (rev. 122110).

Note that when DisableSecureEventInput() is called, the internal flag is cleared: _data->_inSecureInputState = NO.

However later in the method when EnableSecureEventInput() is called, the internal flag is untouched. Same for the later call to DisableSecureEventInput().

This could lead to a situation where secure event input is enabled, but _data->_inSecureInputState is NO. If this method is called again, it will not disable secure event input because _data->_inSecureInputState is NO and so the initial test fails and secure input remains enabled.

Secure input remaining enabled is a problem for apps such as TextExpander:

http://smilesoftware.com/TextExpander/secureinput.html

Here's info from Apple on how to handle secure input:

http://developer.apple.com/library/mac/#technotes/tn2150/

We have a number of reports that this problem is common with Chrome. It's not common with Safari, but perhaps Safari has a higher-level workaround.

I'm not a member of the WebKit project, and I'm afraid I don't have a reproducible case to trigger the logic problem I'm positing here, but I'm filing this bug in hopes that I can interest someone on the dev team or with commit privileges to consider making a fix.

Here's the code I suggest. The only changes are to update _data->_inSecureInputState after the calls to EnableSecureEventInput() and DisableSecureEventInput() later in this method:

- (void)_updateSecureInputState
{
    if (![[self window] isKeyWindow] || ![self _isFocused]) {
        if (_data->_inSecureInputState) {
            DisableSecureEventInput();
            _data->_inSecureInputState = NO;
        }
        return;
    }
    // WKView has a single input context for all editable areas (except for plug-ins).
    NSTextInputContext *context = [super inputContext];
    bool isInPasswordField = _data->_page->editorState().isInPasswordField;

    if (isInPasswordField) {
        if (!_data->_inSecureInputState) {
            EnableSecureEventInput();
            _data->_inSecureInputState = YES;
        }
        static NSArray *romanInputSources = [[NSArray alloc] initWithObjects:&NSAllRomanInputSourcesLocaleIdentifier count:1];
        [context setAllowedInputSourceLocales:romanInputSources];
    } else {
        if (_data->_inSecureInputState) {
            DisableSecureEventInput();
            _data->_inSecureInputState = NO;
        }
        [context setAllowedInputSourceLocales:nil];
    }
    _data->_inSecureInputState = isInPasswordField;
}

[Alexey Proskuryakov]

> We have a number of reports that this problem is common with Chrome. It's not common with Safari, but perhaps Safari has a higher-level workaround.

This is confusing. The code is only used in Safari, not in Chrome or any other Mac applications. This code is in WebKit2 framework, while Chrome has a completely custom implementation, and regular Mac applications use the original WebKit framework.

> However later in the method when EnableSecureEventInput() is called, the internal flag is untouched

This observation is not true. The flag is reset in the last line of the function:

    _data->_inSecureInputState = isInPasswordField;

I don't see a bug in this webkit.org code.

[Alexey Proskuryakov]

I'm not quite sure where Chrome code responsible for secure event mode is, I's start with filing a bug at <http://code.google.com/p/chromium/>.