vcl/inc/osx/salframeview.h | 12 ++
vcl/osx/salframe.cxx | 12 ++
vcl/osx/salframeview.mm | 184 ++++++++++++++++++++++++++++++++++++++++-----
vcl/osx/salnsmenu.mm | 14 +++
vcl/osx/vclnsapp.mm | 11 ++
5 files changed, 212 insertions(+), 21 deletions(-)
New commits:
commit 9ee57f36e26373ee7144d076c93c3462c4fc7110
Author: Patrick Luby <plub...@neooffice.org>
AuthorDate: Sat Dec 3 13:41:23 2022 -0500
Commit: Caolán McNamara <caol...@redhat.com>
CommitDate: Thu Dec 8 09:33:38 2022 +0000
tdf#82115 Commit uncommitted text when a popup menu is opened
The Windows implementation of the SalFrame::EndExtTextInput() method
commits or discards the native input method session. It appears that most macOS
applications discard the uncommitted text when cancelling a session so always
commit the uncommitted text. This change also commits any uncommitted text and
cancels the native input method session whenever a window loses focus like in
Safari, Firefox, and Excel.
Note: if there is any marked text, SalEvent::EndExtTextInput may leave the
cursor hidden so commit the marked range to force the cursor to be visible.
Dispatching SalEvent::EndExtTextInput in SalFrame::EndExtTextInput()
creates some other related native input method handling bugs that have also
been fixed in this patch:
- Whenever a SalEvent::EndExtTextInput event is dispatched, cancel the
native input method session so that the native input context's state is in sync
with LibreOffice's internal state. The only exceptions are in [SalFrameView
insertText:replacementRange:] or [SalFrameView
setMarkedText:selectedRange:replacementRange:] because when these two selectors
commit text, the native input method session has already been cancelled and
calling [SalFrameView endExtTextInput] will cause repeated text insertions from
the native macOS Character Viewer dialog to overwrite previous insertions.
- Highlight all characters in the selected range. Normally uncommitted text
is underlined but when an item is selected in the native input method popup or
selecting a subblock of uncommitted text using the left or right arrow keys,
the selection range is set and the selected range is either highlighted like in
Excel or is bold underlined like in Safari. Highlighting the selected range was
chosen because highlighting was used in LibreOffice 7.4.x and using bold and
double underlines can get clipped making the selection range indistinguishable
from the rest of the uncommitted text.
- The fix for ooo#106901 always returns a zero length range if [self
markedRange] called outside of mbInKeyInput. If a zero length range is
returned, macOS won't call [self firstRectForCharacterRange:actualRange:] for
any newly appended uncommitted text and the native input method popup will
appear in the bottom left corner of the screen. So, [self markedRange] now
returns the marked range if is valid.
- Commit uncommitted text before dispatching menu item selections and key
shortcuts. In certain cases such as selecting the Insert > Comment menu item or
pressing Command-Option-C in a Writer document while there is uncommitted text
will call AquaSalFrame::EndExtTextInput() which will dispatch a
SalEvent::EndExtTextInput event. Writer's handler for that event will delete
the uncommitted text and then insert the committed text but LibreOffice will
crash when deleting the uncommitted text because deletion of the text also
removes and deletes the newly inserted comment.
Change-Id: I4ff4682aeef7d42ce26059aa76f971a68128833c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143619
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caol...@redhat.com>
diff --git a/vcl/inc/osx/salframeview.h b/vcl/inc/osx/salframeview.h
index 36537f3db958..4ec0b6c06651 100644
--- a/vcl/inc/osx/salframeview.h
+++ b/vcl/inc/osx/salframeview.h
@@ -59,6 +59,9 @@ enum class SalEvent;
-(void)registerDraggingDestinationHandler:(id)theHandler;
-(void)unregisterDraggingDestinationHandler:(id)theHandler;
+
+-(void)endExtTextInput;
+-(void)endExtTextInput:(EndExtTextInputFlags)nFlags;
@end
@interface SalFrameView : AquaA11yWrapper <NSTextInputClient>
@@ -79,9 +82,14 @@ enum class SalEvent;
// #i102807# used by magnify event handler
NSTimeInterval mfLastMagnifyTime;
float mfMagnifyDeltaSum;
+
+ BOOL mbInEndExtTextInput;
+ BOOL mbInCommitMarkedText;
+ NSAttributedString* mpLastMarkedText;
}
+(void)unsetMouseFrame: (AquaSalFrame*)pFrame;
-(id)initWithSalFrame: (AquaSalFrame*)pFrame;
+-(void)dealloc;
-(AquaSalFrame*)getSalFrame;
-(BOOL)acceptsFirstResponder;
-(BOOL)acceptsFirstMouse: (NSEvent *)pEvent;
@@ -112,6 +120,7 @@ enum class SalEvent;
-(BOOL)sendSingleCharacter:(NSEvent*)pEvent;
-(BOOL)handleKeyDownException:(NSEvent*)pEvent;
-(void)clearLastEvent;
+-(void)clearLastMarkedText;
/*
text action methods
*/
@@ -206,6 +215,9 @@ enum class SalEvent;
-(void)registerDraggingDestinationHandler:(id)theHandler;
-(void)unregisterDraggingDestinationHandler:(id)theHandler;
+-(void)endExtTextInput;
+-(void)endExtTextInput:(EndExtTextInputFlags)nFlags;
+
@end
#endif // INCLUDED_VCL_INC_OSX_SALFRAMEVIEW_H
diff --git a/vcl/osx/salframe.cxx b/vcl/osx/salframe.cxx
index 77ad16bd3f0d..330a6c041f6a 100644
--- a/vcl/osx/salframe.cxx
+++ b/vcl/osx/salframe.cxx
@@ -1012,8 +1012,16 @@ void AquaSalFrame::SetInputContext( SalInputContext*
pContext )
return;
}
-void AquaSalFrame::EndExtTextInput( EndExtTextInputFlags )
-{
+void AquaSalFrame::EndExtTextInput( EndExtTextInputFlags nFlags )
+{
+ // tdf#82115 Commit uncommitted text when a popup menu is opened
+ // The Windows implementation of this method commits or discards the native
+ // input method session. It appears that very few, if any, macOS
+ // applications discard the uncommitted text when cancelling a session so
+ // always commit the uncommitted text.
+ SalFrameWindow *pWindow = static_cast<SalFrameWindow*>(mpNSWindow);
+ if (pWindow && [pWindow isKindOfClass:[SalFrameWindow class]])
+ [pWindow endExtTextInput:nFlags];
}
OUString AquaSalFrame::GetKeyName( sal_uInt16 nKeyCode )
diff --git a/vcl/osx/salframeview.mm b/vcl/osx/salframeview.mm
index 40a06ddd54ad..289f74aad3c6 100644
--- a/vcl/osx/salframeview.mm
+++ b/vcl/osx/salframeview.mm
@@ -265,6 +265,10 @@ static AquaSalFrame* getMouseContainerFrame()
mpFrame->CallCallback( SalEvent::GetFocus, nullptr );
mpFrame->SendPaintEvent(); // repaint controls as active
}
+
+ // Prevent the same native input method popup that was cancelled in a
+ // previous call to [self windowDidResignKey:] from reappearing
+ [self endExtTextInput];
}
-(void)windowDidResignKey: (NSNotification*)pNotification
@@ -272,6 +276,10 @@ static AquaSalFrame* getMouseContainerFrame()
(void)pNotification;
SolarMutexGuard aGuard;
+ // Commit any uncommitted text and cancel the native input method session
+ // whenever a window loses focus like in Safari, Firefox, and Excel
+ [self endExtTextInput];
+
if( mpFrame && AquaSalFrame::isAlive( mpFrame ) )
{
mpFrame->CallCallback(SalEvent::LoseFocus, nullptr);
@@ -348,7 +356,7 @@ static AquaSalFrame* getMouseContainerFrame()
if( mpFrame && AquaSalFrame::isAlive( mpFrame ) )
{
// #i84461# end possible input
- mpFrame->CallCallback( SalEvent::EndExtTextInput, nullptr );
+ [self endExtTextInput];
if( AquaSalFrame::isAlive( mpFrame ) )
{
mpFrame->CallCallback( SalEvent::Close, nullptr );
@@ -437,6 +445,18 @@ static AquaSalFrame* getMouseContainerFrame()
mDraggingDestinationHandler = nil;
}
+-(void)endExtTextInput
+{
+ [self endExtTextInput:EndExtTextInputFlags::Complete];
+}
+
+-(void)endExtTextInput:(EndExtTextInputFlags)nFlags
+{
+ SalFrameView *pView = static_cast<SalFrameView*>([self firstResponder]);
+ if (pView && [pView isKindOfClass:[SalFrameView class]])
+ [pView endExtTextInput:nFlags];
+}
+
@end
@implementation SalFrameView
@@ -452,17 +472,30 @@ static AquaSalFrame* getMouseContainerFrame()
{
mDraggingDestinationHandler = nil;
mpFrame = pFrame;
+ mpLastEvent = nil;
mMarkedRange = NSMakeRange(NSNotFound, 0);
mSelectedRange = NSMakeRange(NSNotFound, 0);
mpReferenceWrapper = nil;
mpMouseEventListener = nil;
mpLastSuperEvent = nil;
+ mfLastMagnifyTime = 0.0;
+
+ mbInEndExtTextInput = NO;
+ mbInCommitMarkedText = NO;
+ mpLastMarkedText = nil;
}
- mfLastMagnifyTime = 0.0;
return self;
}
+-(void)dealloc
+{
+ [self clearLastEvent];
+ [self clearLastMarkedText];
+
+ [super dealloc];
+}
+
-(AquaSalFrame*)getSalFrame
{
return mpFrame;
@@ -922,7 +955,11 @@ static AquaSalFrame* getMouseContainerFrame()
if( AquaSalFrame::isAlive( mpFrame ) )
{
- mpLastEvent = pEvent;
+ // Retain the event as it will be released sometime before a key up
+ // event is dispatched
+ [self clearLastEvent];
+ mpLastEvent = [pEvent retain];
+
mbInKeyInput = true;
mbNeedSpecialKeyHandle = false;
mbKeyHandled = false;
@@ -978,10 +1015,20 @@ static AquaSalFrame* getMouseContainerFrame()
SolarMutexGuard aGuard;
+ // Ignore duplicate events that are sometimes posted during cancellation
+ // of the native input method session. This usually happens when
+ // [self endExtTextInput] is called from [self windowDidBecomeKey:] and,
+ // if the native input method popup, that was cancelled in a
+ // previous call to [self windowDidResignKey:], has reappeared. In such
+ // cases, the native input context posts the reappearing popup's
+ // uncommitted text.
+ if (mbInEndExtTextInput && !mbInCommitMarkedText)
+ return;
+
if( AquaSalFrame::isAlive( mpFrame ) )
{
NSString* pInsert = nil;
- if( [aString isMemberOfClass: [NSAttributedString class]] )
+ if( [aString isKindOfClass: [NSAttributedString class]] )
pInsert = [aString string];
else
pInsert = aString;
@@ -1527,9 +1574,14 @@ static AquaSalFrame* getMouseContainerFrame()
// of the keyboard viewer. For unknown reasons having no marked range
// in this case causes a crash. So we say we have a marked range anyway
// This is a hack, since it is not understood what a) causes that crash
- // and b) why we should have a marked range at this point.
+ // and b) why we should have a marked range at this point. Stop the native
+ // input method popup from appearing in the bottom left corner of the
+ // screen by returning the marked range if is valid when called outside of
+ // mbInKeyInput. If a zero length range is returned, macOS won't call
+ // [self firstRectForCharacterRange:actualRange:] for any newly appended
+ // uncommitted text.
if( ! mbInKeyInput )
- return NSMakeRange( 0, 0 );
+ return mMarkedRange.location != NSNotFound ? mMarkedRange :
NSMakeRange( 0, 0 );
return [self hasMarkedText] ? mMarkedRange : NSMakeRange( NSNotFound, 0 );
}
@@ -1547,26 +1599,49 @@ static AquaSalFrame* getMouseContainerFrame()
if( ![aString isKindOfClass:[NSAttributedString class]] )
aString = [[[NSAttributedString alloc] initWithString:aString]
autorelease];
- NSRange rangeToReplace = [self hasMarkedText] ? [self markedRange] : [self
selectedRange];
- if( rangeToReplace.location == NSNotFound )
- {
- mMarkedRange = NSMakeRange( selRange.location, [aString length] );
- mSelectedRange = NSMakeRange( selRange.location, selRange.length );
- }
- else
- {
- mMarkedRange = NSMakeRange( rangeToReplace.location, [aString length]
);
- mSelectedRange = NSMakeRange( rangeToReplace.location +
selRange.location, selRange.length );
- }
+
+ // Reset cached state
+ [self unmarkText];
int len = [aString length];
SalExtTextInputEvent aInputEvent;
if( len > 0 ) {
+ // Set the marked and selected ranges to the marked text and selected
+ // range parameters
+ mMarkedRange = NSMakeRange( 0, [aString length] );
+ if (selRange.location == NSNotFound || selRange.location >=
mMarkedRange.length)
+ mSelectedRange = NSMakeRange( NSNotFound, 0 );
+ else
+ mSelectedRange = NSMakeRange( selRange.location,
selRange.location + selRange.length > mMarkedRange.length ? mMarkedRange.length
- selRange.location : selRange.length );
+
+ // If we are going to post uncommitted text, cache the string paramater
+ // as is needed in both [self endExtTextInput] and
+ // [self attributedSubstringForProposedRange:actualRange:]
+ mpLastMarkedText = [aString retain];
+
NSString *pString = [aString string];
OUString aInsertString( GetOUString( pString ) );
std::vector<ExtTextInputAttr> aInputFlags( std::max( 1, len ),
ExtTextInputAttr::NONE );
+ int nSelectionStart = (mSelectedRange.location == NSNotFound ? len :
mSelectedRange.location);
+ int nSelectionEnd = (mSelectedRange.location == NSNotFound ? len :
mSelectedRange.location + selRange.length);
for ( int i = 0; i < len; i++ )
{
+ // Highlight all characters in the selected range. Normally
+ // uncommitted text is underlined but when an item is selected in
+ // the native input method popup or selecting a subblock of
+ // uncommitted text using the left or right arrow keys, the
+ // selection range is set and the selected range is either
+ // highlighted like in Excel or is bold underlined like in
+ // Safari. Highlighting the selected range was chosen because
+ // using bold and double underlines can get clipped making the
+ // selection range indistinguishable from the rest of the
+ // uncommitted text.
+ if (i >= nSelectionStart && i < nSelectionEnd)
+ {
+ aInputFlags[i] = ExtTextInputAttr::Highlight;
+ continue;
+ }
+
unsigned int nUnderlineValue;
NSRange effectiveRange;
@@ -1590,7 +1665,7 @@ static AquaSalFrame* getMouseContainerFrame()
}
aInputEvent.maText = aInsertString;
- aInputEvent.mnCursorPos = selRange.location;
+ aInputEvent.mnCursorPos = nSelectionStart;
aInputEvent.mpTextAttr = aInputFlags.data();
mpFrame->CallCallback( SalEvent::ExtTextInput, static_cast<void
*>(&aInputEvent) );
} else {
@@ -1606,6 +1681,8 @@ static AquaSalFrame* getMouseContainerFrame()
- (void)unmarkText
{
+ [self clearLastMarkedText];
+
mSelectedRange = mMarkedRange = NSMakeRange(NSNotFound, 0);
}
@@ -1650,7 +1727,20 @@ static AquaSalFrame* getMouseContainerFrame()
-(void)clearLastEvent
{
- mpLastEvent = nil;
+ if (mpLastEvent)
+ {
+ [mpLastEvent release];
+ mpLastEvent = nil;
+ }
+}
+
+-(void)clearLastMarkedText
+{
+ if (mpLastMarkedText)
+ {
+ [mpLastMarkedText release];
+ mpLastMarkedText = nil;
+ }
}
- (NSRect)firstRectForCharacterRange:(NSRange)aRange
actualRange:(NSRangePointer)actualRange
@@ -1752,6 +1842,62 @@ static AquaSalFrame* getMouseContainerFrame()
mDraggingDestinationHandler = nil;
}
+-(void)endExtTextInput
+{
+ [self endExtTextInput:EndExtTextInputFlags::Complete];
+}
+
+-(void)endExtTextInput:(EndExtTextInputFlags)nFlags
+{
+ // Prevent recursion from any additional [self insertText:] calls that
+ // may be called when cancelling the native input method session
+ if (mbInEndExtTextInput)
+ return;
+
+ mbInEndExtTextInput = YES;
+
+ SolarMutexGuard aGuard;
+
+ NSTextInputContext *pInputContext = [NSTextInputContext
currentInputContext];
+ if (pInputContext)
+ {
+ // Cancel the native input method session
+ [pInputContext discardMarkedText];
+
+ // Commit any uncommitted text. Note: when the delete key is used to
+ // remove all uncommitted characters, the marked range will be zero
+ // length but a SalEvent::EndExtTextInput must still be dispatched.
+ if (mpLastMarkedText && [mpLastMarkedText length] &&
mMarkedRange.location != NSNotFound && mpFrame &&
AquaSalFrame::isAlive(mpFrame))
+ {
+ // If there is any marked text, SalEvent::EndExtTextInput may leave
+ // the cursor hidden so commit the marked text to force the cursor
+ // to be visible.
+ mbInCommitMarkedText = YES;
+ if (nFlags & EndExtTextInputFlags::Complete)
+ [self insertText:mpLastMarkedText
replacementRange:NSMakeRange(0, [mpLastMarkedText length])];
+ else
+ [self insertText:[NSString string]
replacementRange:NSMakeRange(0, 0)];
+ mbInCommitMarkedText = NO;
+ }
+
+ [self unmarkText];
+
+ // If a different view is the input context's client, commit that
+ // view's uncommitted text as well
+ id<NSTextInputClient> pClient = [pInputContext client];
+ if (pClient != self)
+ {
+ SalFrameView *pView = static_cast<SalFrameView*>(pClient);
+ if ([pView isKindOfClass:[SalFrameView class]])
+ [pView endExtTextInput];
+ else
+ [pClient unmarkText];
+ }
+ }
+
+ mbInEndExtTextInput = NO;
+}
+
@end
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/osx/salnsmenu.mm b/vcl/osx/salnsmenu.mm
index 546b47601325..b2df2da7e5f5 100644
--- a/vcl/osx/salnsmenu.mm
+++ b/vcl/osx/salnsmenu.mm
@@ -25,6 +25,7 @@
#include <osx/salinst.h>
#include <osx/saldata.hxx>
#include <osx/salframe.h>
+#include <osx/salframeview.h>
#include <osx/salmenu.h>
#include <osx/salnsmenu.h>
@@ -95,6 +96,19 @@
(void)aSender;
SolarMutexGuard aGuard;
+ // Commit uncommitted text before dispatching the selecting menu item. In
+ // certain cases such as selecting the Insert > Comment menu item in a
+ // Writer document while there is uncommitted text will call
+ // AquaSalFrame::EndExtTextInput() which will dispatch a
+ // SalEvent::EndExtTextInput event. Writer's handler for that event will
+ // delete the uncommitted text and then insert the committed text but
+ // LibreOffice will crash when deleting the uncommitted text because
+ // deletion of the text also removes and deletes the newly inserted
+ // comment.
+ NSWindow* pKeyWin = [NSApp keyWindow];
+ if( pKeyWin && [pKeyWin isKindOfClass: [SalFrameWindow class]] )
+ [static_cast<SalFrameWindow*>(pKeyWin) endExtTextInput];
+
// tdf#49853 Keyboard shortcuts are also handled by the menu bar, but at
least some of them
// must still end up in the view. This is necessary to handle common edit
actions in docked
// windows (e.g. in toolbar fields).
diff --git a/vcl/osx/vclnsapp.mm b/vcl/osx/vclnsapp.mm
index d31b41b5f1e6..5daf923ce105 100644
--- a/vcl/osx/vclnsapp.mm
+++ b/vcl/osx/vclnsapp.mm
@@ -96,6 +96,17 @@
NSWindow* pKeyWin = [NSApp keyWindow];
if( pKeyWin && [pKeyWin isKindOfClass: [SalFrameWindow class]] )
{
+ // Commit uncommitted text before dispatching key shortcuts. In
+ // certain cases such as pressing Command-Option-C in a Writer
+ // document while there is uncommitted text will call
+ // AquaSalFrame::EndExtTextInput() which will dispatch a
+ // SalEvent::EndExtTextInput event. Writer's handler for that event
+ // will delete the uncommitted text and then insert the committed
+ // text but LibreOffice will crash when deleting the uncommitted
+ // text because deletion of the text also removes and deletes the
+ // newly inserted comment.
+ [static_cast<SalFrameWindow*>(pKeyWin) endExtTextInput];
+
AquaSalFrame* pFrame = [static_cast<SalFrameWindow*>(pKeyWin)
getSalFrame];
unsigned int nModMask = ([pEvent modifierFlags] &
(NSEventModifierFlagShift|NSEventModifierFlagControl|NSEventModifierFlagOption|NSEventModifierFlagCommand));
/*
