TextEventDispatcher should replace native line breaker in composition string with XP line breaker even if it's set by TextInputProcessor

RESOLVED FIXED in Firefox 55

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: tpi:+)

Attachments

(4 attachments)

Currently, TextEventDispatcher replaces \r\n in composition string at:

1. Dispatching commit event caused by everyone.
2. Dispatching composition change event caused by native IME handlers.

However, this should be testable with automated tests and anyway, doesn't make sense TextInputProcessor to input native line breakers with composition string. So, I think that TextEventDispatcher should always replace \r\n with \n even if the composition is caused by TextInputProcessor.
Priority: -- → P3
Whiteboard: tpi:+
Comment on attachment 8847877 [details]
Bug 1347433 part.1 Separate TextRange offset and length adjustment to AdjustRange()

https://reviewboard.mozilla.org/r/120800/#review122828
Attachment #8847877 - Flags: review?(m_kato) → review+
Comment on attachment 8847878 [details]
Bug 1347433 part.2 Implement TextEventDispatcher::PendingComposition::ReplaceNativeLineBreakers() and TextEventDispatcher::PendingComposition::Set() should use it

https://reviewboard.mozilla.org/r/120802/#review122830
Attachment #8847878 - Flags: review?(m_kato) → review+
Comment on attachment 8847879 [details]
Bug 1347433 part.3 TextEventDispatcher::PendingComposition::Flush() should replace native line breakers in the composition string before dispatching composition event

https://reviewboard.mozilla.org/r/120804/#review122834
Attachment #8847879 - Flags: review?(m_kato) → review+
Comment on attachment 8847880 [details]
Bug 1347433 part.4 Add automated tests to check if \n and \r\n in composition string are treated as expected

https://reviewboard.mozilla.org/r/120806/#review122864
Attachment #8847880 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/a77b4a61225a
part.1 Separate TextRange offset and length adjustment to AdjustRange() r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c27841632c8a
part.2 Implement TextEventDispatcher::PendingComposition::ReplaceNativeLineBreakers() and TextEventDispatcher::PendingComposition::Set() should use it r=m_kato
https://hg.mozilla.org/integration/autoland/rev/eaf379d6fc13
part.3 TextEventDispatcher::PendingComposition::Flush() should replace native line breakers in the composition string before dispatching composition event r=m_kato
https://hg.mozilla.org/integration/autoland/rev/9339469b766a
part.4 Add automated tests to check if \n and \r\n in composition string are treated as expected r=m_kato
You need to log in before you can comment on or make changes to this bug.