Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mac): restore OSK keys that became invisible in mac OS 14 Sonoma #11388

Merged
merged 2 commits into from
May 13, 2024

Conversation

sgschantz
Copy link
Contributor

@sgschantz sgschantz commented May 7, 2024

According to AppKit Release Notes for macOS 14 the property NSView.clipsToBounds which was defaulted to true was changed to default to false. This caused the Keyman OSK keys to not be drawn correctly under Sonoma, giving the appearance of a blank OSK. This change is to override the new default behavior and set the property back to true.

Fixes #11379

Also, with the move to Xcode 15.3, there are several APIs called from Keyman for Mac that were deprecated before our minimum supported version of macOS (now set to 10.13). These can be replaced without doing version checks because the replacements are available for all of our supported OS versions. It was suspected that these may have caused the OSK issue, but they had no effect.

To support the debugging of the OSK issue, log statements were also added using the Unified Logging API for which we now have support in our minimum target version of macOS.

User Testing

  • TEST_OSK_KEYS_VISIBLE: confirm that the OSK looks as expected
  1. Select On-screen Keyboard from the Keyman menu
  2. Confirm that the keyboard is not blank but populated with dozens of keys as expected
  • TEST_OSK_VIEW_RESIZABLE: confirm that the OSK is resizable
  1. Select On-screen Keyboard from the Keyman menu
  2. Resize the OSK
  3. Confirm that the keys resize appropriately and are still visible

With the move to Xcode 15.3, there are many APIs called from
Keyman for Mac that were deprecated before our minimum supported
version of macOS (now set to 10.13)
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label May 7, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented May 7, 2024

User Test Results

Test specification and instructions

  • TEST_OSK_KEYS_VISIBLE (PASSED): I tested this issue with the attached "Keyman 17.0.320-beta-test-11388" build on an macOS environment: Here is my observation. (notes)
  • TEST_OSK_VIEW_RESIZABLE (PASSED) (notes)

Test Artifacts

@sgschantz sgschantz self-assigned this May 7, 2024
@github-actions github-actions bot added the mac/ label May 7, 2024
@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S1 milestone May 7, 2024
@github-actions github-actions bot added the fix label May 7, 2024
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label May 7, 2024
set clipsToBounds to true for osk and osk key views
This property began defaulting to false with the update to
macOS 14 sonoma causing the osk keys to become so large
that they obscured the entire osk view.
Also replaced and few more deprecated properties and functions.
@sgschantz sgschantz linked an issue May 10, 2024 that may be closed by this pull request
8 tasks
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels May 10, 2024
@sgschantz sgschantz changed the title fix(mac): replace deprecated API calls fix(mac): restore OSK keys that became invisible in mac OS 14 Sonoma May 10, 2024
@sgschantz sgschantz marked this pull request as ready for review May 10, 2024 08:25
@sgschantz sgschantz requested a review from SabineSIL as a code owner May 10, 2024 08:25
self = [super initWithFrame:frame];
if (self) {
os_log_with_type(oskKeyLog, OS_LOG_TYPE_DEBUG, "KeyView initWithFrame: %{public}@, bounds: %{public}@, default clipsToBounds %{public}@", NSStringFromRect(frame), NSStringFromRect(self.bounds), self.clipsToBounds?@"YES":@"NO");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it the new logs are intentionally being left in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am planning to include more log statements with the unified logging APIs because attaching the debugger to the input method is not practical. I will do this throughout the app for #10997.

With log statements of type DEBUG, the messages are not persisted and are immediately discarded unless debug messages are enabled (disabled by default) and the log is actually being streamed.

}

return self;
}

- (void)drawRect:(NSRect)rect {
os_log_t oskKeyLog = os_log_create("org.sil.keyman", "osk-key");
os_log_with_type(oskKeyLog, OS_LOG_TYPE_DEBUG, "KeyView drawRect: %{public}@, bounds: %{public}@, keyCode: 0x%lx, caption: %{public}@, label: %{public}@", NSStringFromRect(rect), NSStringFromRect(self.bounds), self.keyCode, self.caption.stringValue, self.label.stringValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More "new logs". This one looks like we're making a new logging object... with the same ID as the log noted for the previous comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The os_log_create does not actually create a new log object if it has already been called. Internally, the logging framework creates a singleton for each log object of a certain subsystem/category.

To reduce clutter, I can create a private member variable to hold this reference so that within each method there is only one line of code to write a log statement. It will have no effect on performance, but could make it easier to read the code.

@dinakaranr
Copy link

Test Results

  • TEST_OSK_KEYS_VISIBLE (PASSED): I tested this issue with the attached "Keyman 17.0.320-beta-test-11388" build on an macOS environment: Here is my observation.
    Steps to reproduce:
  1. Install the latest version of the Keyman-17.0.320.dmg file on 10/05/2024
  2. Downloaded and installed the gff_amharic keyboard using the Keyman Configuration dialog.
  3. Select the "Amharic" keyboard on the toolbar.
  4. Open the "On-Screen Keyboard" window by clicking the Keyman icon.

Here, the on-screen keyboard appeared correctly, and it is not in a blank state now. It is showing the "amhairc" key buttons.

  • TEST_OSK_VIEW_RESIZABLE (PASSED):
    Followed above step 1 to 4
    Resize the “on-screen keyboard”window and observed that the key are showing in visible state.(Keyboard appeared in resize state when reopening the keyboard)
    The OSK is working correctly on macOS and when resize the keyboard screen too. It works good. Thanks.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label May 10, 2024
@darcywong00 darcywong00 modified the milestones: A18S1, A18S2 May 11, 2024
@jahorton
Copy link
Contributor

Have we been able to test this on pre-Sonoma devices as well? Since the issue arose with Sonoma, it'd be wise to verify that we don't break on older macOS as a consequence of the fix.

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Agree that we should test against older operating systems as well

@sgschantz
Copy link
Contributor Author

Tested with Monterey, and the OSK passes both users tests there also.

@sgschantz sgschantz merged commit 3e8a594 into beta May 13, 2024
5 checks passed
@sgschantz sgschantz deleted the fix/mac/11379-sonoma-blank-osk branch May 13, 2024 02:37
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.323-beta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

bug(mac): on-screen keyboard blank for all keyboards
6 participants