Lock clipboard history view if device is locked

[codokie]

The clipboard history view should be accessed only if the device is unlocked
Currently it is possible to enter that view by pressing the clipboard toolbar key, even when the device is locked.
This PR restricts entry to the clipboard history when the device is yet to be unlocked
I suspect this is a side effect of #674.
Previously an empty suggestion strip with hidden pinned keys was displayed on input start if the device is locked.
Now with the "auto show toolbar" setting enabled, the toolbar keys will be shown, which may include the clipboard toolbar key.

[codokie]

I wonder if a toast message should be added because with this change when you click the clipboard key, nothing happens.

[Helium314]

Thanks for noticing!
I really favor small and contained changes, like

    public void setToolbarVisibility(final boolean visible) {
        final KeyguardManager km = (KeyguardManager) getContext().getSystemService(Context.KEYGUARD_SERVICE);
        final boolean locked = Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1
                ? km.isDeviceLocked()
                : km.isKeyguardLocked();
        if (locked) {
            mPinnedKeys.setVisibility(GONE);
            mSuggestionsStrip.setVisibility(VISIBLE);
            mToolbarContainer.setVisibility(GONE);
        } else if (visible) {
            mPinnedKeys.setVisibility(GONE);
            mSuggestionsStrip.setVisibility(GONE);
            mToolbarContainer.setVisibility(VISIBLE);
        } else {
            mToolbarContainer.setVisibility(GONE);
            mSuggestionsStrip.setVisibility(VISIBLE);
            mPinnedKeys.setVisibility(VISIBLE);
        }
        mToolbarExpandKey.setScaleX((visible && !locked ? -1f : 1f) * mRtl);
    }

This seems to work for me, and with this the toolbar when locked should be exactly like before #674.

[codokie]

But I don't think there are toolbar keys that should be hidden except for the clipboard key when the device is locked.
Plus, some keys like the Select all and the right/left arrows are useful to have when typing the device's password.
There really is no need to hide the pinned toolbar keys either if the clipboard history can't be opened.
Lastly, I think the suggestions strip should be hidden if personalized dictionary is enabled and the device is locked.

[Helium314]

Hiding the toolbar is consistent with the previous behavior and it's consistent with the behavior when auto-show is disabled (because the toolbar key click listerner is null).
Further, it's safe regarding any future toolbar key additions, i.e. you'll never need to think about what might happen if the key is shown.

[codokie]

That would be easier & futureproof but I disagree with this approach because the other current keys are useful & harmless.
The expand toolbar key is still nice to have even if it is not touchable because it displays the incognito icon when entering the device password.

[Helium314]

I added my proposed changes so a. the current behavior is consistent and b. I can finally do the beta release I had planned for this weekend.

Ignoring certain keys / codes should still be added, because now with the fully customizable keyboards, users can add any key anywhere.
Ignoring should probably be done by code, in LatinIME or in InputLogic. Not just for clipboard, but also for pasting and possibly for cutting and copying.

[codokie]

Android does not allow pasting the primary clipboard when the device is locked.
There is no need to restrict cutting and copying imo.
Except for the clipboard key, I can't think of any other key that may be problematic when the device is locked.
Regarding 53c66b8, it re-uses code code from updateKeys().
Also while at it, why keep mSuggestionsStrip visible when the device is locked? Anyone could access the personalized dictionary (if enabled) that way.
Should this PR be kept open?

[codokie]

Please re-consider my approach, taking the locked state into account in setToolbarVisibility() makes it difficult to add new toolbar modes (#838).

If you think the toolbar keys should all be hidden when the device is locked (current behavior), then it is preferable to just hide the strip container when the device is locked in KeyboardSwitcher

[Helium314]

Regarding 53c66b8, it re-uses code code from updateKeys().

Huh? What do you mean?

Also while at it, why keep mSuggestionsStrip visible when the device is locked?

What do you mean? To my knowledge I did not change anything compared to behavior before the auto hide/show PR, and this issue is, according to title, about accessiblity of clipboard history and not about the suggestion strip.

Please re-consider my approach, taking the locked state into account in setToolbarVisibility() makes it difficult to add new toolbar modes (#838).

I did my changes before you started the PR, so I was not able to consider that my changes affect your planned PR. I don't think that, on every change I do, I should consider that maybe you will do a PR that might be affected by my changes.

If you think the toolbar keys should all be hidden when the device is locked (current behavior)

I don't have a strong opinion on this, but I certainly want consistent behavior.

[codokie]

What do you mean?

code that needs to be duplicated should be moved to a new function for better maintainability.

from 53c66b8:

        if (locked) {
            mSuggestionsStrip.setVisibility(VISIBLE);

makes no sense to show the suggestions strip when the device is locked and is a privacy concern if personal suggestions are enabled

this issue is, according to title, about accessiblity of clipboard history and not about the suggestion strip.

I could have changed the title so that it would still obey the strict rule of "only 1 thing"..

I don't think that, on every change I do, I should consider that maybe you will do a PR that might be affected by my changes.

I never said that you should.

I certainly want consistent behavior

it seems to me that you take code contributions with more than a grain of salt in the name of consistency and preservation of the current behavior, even if it is flawed

[codokie]

Sorry but I don't think that I want to continue to contribute to this repo, I'm just tired of the nitpicking.
Feel free to close this and the other PRs if you don't like them. Thanks