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

COMPOSE-499 Fix crash with Asian languages in TextField. #872

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,35 @@ internal interface IOSSkikoInput {
*/
fun rangeEnclosingPosition(position: Int, withGranularity: UITextGranularity, inDirection: UITextDirection): IntRange?

/**
* Return whether a text position is at a boundary of a text unit of a specified granularity in a specified direction.
* https://developer.apple.com/documentation/uikit/uitextinputtokenizer/1614553-isposition?language=objc
* @param position
* A text-position object that represents a location in a document.
* @param granularity
* A constant that indicates a certain granularity of text unit.
* @param direction
* A constant that indicates a direction relative to position. The constant can be of type UITextStorageDirection or UITextLayoutDirection.
* @return
* TRUE if the text position is at the given text-unit boundary in the given direction; FALSE if it is not at the boundary.
*/
fun isPositionAtBoundary(position: Int, atBoundary: UITextGranularity, inDirection: UITextDirection): Boolean

/**
* Return whether a text position is within a text unit of a specified granularity in a specified direction.
* https://developer.apple.com/documentation/uikit/uitextinputtokenizer/1614491-isposition?language=objc
* @param position
* A text-position object that represents a location in a document.
* @param granularity
* A constant that indicates a certain granularity of text unit.
* @param direction
* A constant that indicates a direction relative to position. The constant can be of type UITextStorageDirection or UITextLayoutDirection.
* @return
* TRUE if the text position is within a text unit of the specified granularity in the specified direction; otherwise, return FALSE.
* If the text position is at a boundary, return TRUE only if the boundary is part of the text unit in the given direction.
*/
fun isPositionWithingTextUnit(position: Int, withinTextUnit: UITextGranularity, inDirection: UITextDirection): Boolean

object Empty : IOSSkikoInput {
override fun hasText(): Boolean = false
override fun insertText(text: String) = Unit
Expand All @@ -144,5 +173,17 @@ internal interface IOSSkikoInput {
withGranularity: UITextGranularity,
inDirection: UITextDirection
): IntRange? = null

override fun isPositionAtBoundary(
position: Int,
atBoundary: UITextGranularity,
inDirection: UITextDirection
): Boolean = false

override fun isPositionWithingTextUnit(
position: Int,
withinTextUnit: UITextGranularity,
inDirection: UITextDirection
): Boolean = false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -361,16 +361,7 @@ internal class UIKitTextInputService(
}
}

val iterator: BreakIterator =
when (withGranularity) {
UITextGranularity.UITextGranularitySentence -> BreakIterator.makeSentenceInstance()
UITextGranularity.UITextGranularityLine -> BreakIterator.makeLineInstance()
UITextGranularity.UITextGranularityWord -> BreakIterator.makeWordInstance()
UITextGranularity.UITextGranularityCharacter -> BreakIterator.makeCharacterInstance()
UITextGranularity.UITextGranularityParagraph -> TODO("UITextGranularityParagraph iterator")
UITextGranularity.UITextGranularityDocument -> TODO("UITextGranularityDocument iterator")
else -> error("Unknown granularity")
}
val iterator: BreakIterator = withGranularity.toTextIterator()
iterator.setText(text)

if (inDirection == UITextStorageDirectionForward) {
Expand Down Expand Up @@ -399,6 +390,63 @@ internal class UIKitTextInputService(
error("Unknown inDirection: $inDirection")
}
}

/**
* Return whether a text position is at a boundary of a text unit of a specified granularity in a specified direction.
* https://developer.apple.com/documentation/uikit/uitextinputtokenizer/1614553-isposition?language=objc
* @param position
* A text-position object that represents a location in a document.
* @param granularity
* A constant that indicates a certain granularity of text unit.
* @param direction
* A constant that indicates a direction relative to position. The constant can be of type UITextStorageDirection or UITextLayoutDirection.
* @return
* TRUE if the text position is at the given text-unit boundary in the given direction; FALSE if it is not at the boundary.
*/
override fun isPositionAtBoundary(
position: Int,
atBoundary: UITextGranularity,
inDirection: UITextDirection
): Boolean {
val text = getState()?.text ?: return false
assert(position >= 0) { "isPositionAtBoundary position >= 0" }

val iterator = atBoundary.toTextIterator()
iterator.setText(text)
return iterator.isBoundary(position)
}

/**
* Return whether a text position is within a text unit of a specified granularity in a specified direction.
* https://developer.apple.com/documentation/uikit/uitextinputtokenizer/1614491-isposition?language=objc
* @param position
* A text-position object that represents a location in a document.
* @param granularity
* A constant that indicates a certain granularity of text unit.
* @param direction
* A constant that indicates a direction relative to position. The constant can be of type UITextStorageDirection or UITextLayoutDirection.
* @return
* TRUE if the text position is within a text unit of the specified granularity in the specified direction; otherwise, return FALSE.
* If the text position is at a boundary, return TRUE only if the boundary is part of the text unit in the given direction.
*/
override fun isPositionWithingTextUnit(
position: Int,
withinTextUnit: UITextGranularity,
inDirection: UITextDirection
): Boolean {
val text = getState()?.text ?: return false
assert(position >= 0) { "isPositionWithingTextUnit position >= 0" }

val iterator = withinTextUnit.toTextIterator()
iterator.setText(text)

if (inDirection == UITextStorageDirectionForward) {

} else if (inDirection == UITextStorageDirectionBackward) {

}
return false // TODO: Write implementation
}
}

val skikoUITextInputTraits = object : SkikoUITextInputTraits {
Expand Down Expand Up @@ -557,3 +605,14 @@ internal class UIKitTextInputService(
private fun getState(): TextFieldValue? = currentInput?.value

}

private fun UITextGranularity.toTextIterator() =
when (this) {
UITextGranularity.UITextGranularitySentence -> BreakIterator.makeSentenceInstance()
UITextGranularity.UITextGranularityLine -> BreakIterator.makeLineInstance()
UITextGranularity.UITextGranularityWord -> BreakIterator.makeWordInstance()
UITextGranularity.UITextGranularityCharacter -> BreakIterator.makeCharacterInstance()
UITextGranularity.UITextGranularityParagraph -> TODO("UITextGranularityParagraph iterator")
UITextGranularity.UITextGranularityDocument -> TODO("UITextGranularityDocument iterator")
else -> error("Unknown granularity")
}
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,13 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
}

override fun offsetFromPosition(from: UITextPosition, toPosition: UITextPosition): NSInteger {
val fromPosition = from as IntermediateTextPosition
val to = toPosition as IntermediateTextPosition
return to.position - fromPosition.position
if (from !is IntermediateTextPosition) {
error("from !is IntermediateTextPosition")
}
if (toPosition !is IntermediateTextPosition) {
error("toPosition !is IntermediateTextPosition")
}
return toPosition.position - from.position
}

override fun tokenizer(): UITextInputTokenizerProtocol = @Suppress("CONFLICTING_OVERLOADS") object : UITextInputStringTokenizer() {
Expand All @@ -515,10 +519,19 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
* TRUE if the text position is at the given text-unit boundary in the given direction; FALSE if it is not at the boundary.
*/
override fun isPosition(
position: UITextPosition,
position: UITextPosition, // Attention! position may be null.
atBoundary: UITextGranularity,
inDirection: UITextDirection
): Boolean = TODO("implement isPosition")
): Boolean {
if (position !is IntermediateTextPosition) {
return false
}
return input?.isPositionAtBoundary(
position = position.position.toInt(),
atBoundary = atBoundary,
inDirection = inDirection,
) ?: false
}

/**
* Return whether a text position is within a text unit of a specified granularity in a specified direction.
Expand All @@ -534,10 +547,19 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
* If the text position is at a boundary, return TRUE only if the boundary is part of the text unit in the given direction.
*/
override fun isPosition(
position: UITextPosition,
position: UITextPosition, // Attention! position may be null.
withinTextUnit: UITextGranularity,
inDirection: UITextDirection
): Boolean = TODO("implement isPosition")
): Boolean {
if (position !is IntermediateTextPosition) {
return false
}
return input?.isPositionWithingTextUnit(
position = position.position.toInt(),
withinTextUnit = withinTextUnit,
inDirection = inDirection,
) ?: false
}

/**
* Return the next text position at a boundary of a text unit of the given granularity in a given direction.
Expand All @@ -555,7 +577,12 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
position: UITextPosition,
toBoundary: UITextGranularity,
inDirection: UITextDirection
): UITextPosition? = null
): UITextPosition? {
return null
if (position !is IntermediateTextPosition) {
Copy link
Member

Choose a reason for hiding this comment

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

The code after return will be never executed. There are a few similar places in this file

Copy link
Author

Choose a reason for hiding this comment

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

Yes. It is intented. Currently this function doesn't have implementation and returns just a stub. When we will write implementation we should check position argument.

error("position !is IntermediateTextPosition")
}
}

/**
* Return the range for the text enclosing a text position in a text unit of a given granularity in a given direction.
Expand All @@ -575,7 +602,9 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
withGranularity: UITextGranularity,
inDirection: UITextDirection
): UITextRange? {
position as IntermediateTextPosition
if (position !is IntermediateTextPosition) {
error("position !is IntermediateTextPosition")
}
return input?.rangeEnclosingPosition(
position = position.position.toInt(),
withGranularity = withGranularity,
Expand All @@ -595,6 +624,9 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
position: UITextPosition,
inDirection: UITextLayoutDirection
): UITextRange? {
if (position !is IntermediateTextPosition) {
error("position !is IntermediateTextPosition")
}
TODO("characterRangeByExtendingPosition, inDirection: ${inDirection.directionToStr()}")
}

Expand All @@ -603,6 +635,9 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
inDirection: UITextStorageDirection
): NSWritingDirection {
return NSWritingDirectionLeftToRight // TODO support RTL text direction
if (position !is IntermediateTextPosition) {
error("position !is IntermediateTextPosition")
}
}

override fun setBaseWritingDirection(writingDirection: NSWritingDirection, forRange: UITextRange) {
Expand All @@ -618,6 +653,9 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
Ideally, here should be correct rect for caret from Compose.
*/
return CGRectMake(x = 1.0, y = 1.0, width = 1.0, height = 1.0)
if (position !is IntermediateTextPosition) {
error("position !is IntermediateTextPosition")
}
}

override fun selectionRectsForRange(range: UITextRange): List<*> = listOf<UITextSelectionRect>()
Expand All @@ -627,9 +665,15 @@ internal class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {

override fun textStylingAtPosition(position: UITextPosition, inDirection: UITextStorageDirection): Map<Any?, *>? {
return NSDictionary.dictionary()
if (position !is IntermediateTextPosition) {
error("position !is IntermediateTextPosition")
}
}

override fun characterOffsetOfPosition(position: UITextPosition, withinRange: UITextRange): NSInteger {
if (position !is IntermediateTextPosition) {
error("position !is IntermediateTextPosition")
}
TODO("characterOffsetOfPosition")
}

Expand Down