Closed
Bug 1339331
Opened 7 years ago
Closed 7 years ago
[TSF] new line is ignored when use ATOK's template input function
Categories
(Core :: Widget: Win32, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: ayakawa.m, Assigned: masayuki)
References
Details
(Keywords: inputmethod, regression, Whiteboard: tpi:+)
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0 Build ID: 20170213030206 Steps to reproduce: 1.enter Editable area(for example, bugzilla comment field) 2.atok on( I use atok2017) 3.push Ctrl+Shift+F11(default assigned template input:定型文章入力) 4.select sentence with new line Actual results: Line breaks in fixed form sentences are ignored Expected results: Line breaks in fixed form sentences are reflected properly
Assignee | ||
Comment 1•7 years ago
|
||
What happens if you make dom.compositionevent.allow_control_characters false? (I guess you need to restart Firefox to enable the pref change.)
Flags: needinfo?(ayakawa.m)
Keywords: inputmethod
Assignee | ||
Comment 2•7 years ago
|
||
Oops, I meant making it true.
Reporter | ||
Comment 3•7 years ago
|
||
change dom.compositionevent.allow_control_characters to TRUE, and I tried again. Same result. New line is ignored.
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Takeshi Ichimaru(a.k.a. Ayakawa) from comment #3) > change dom.compositionevent.allow_control_characters to TRUE, and I tried > again. > Same result. New line is ignored. Thank you. I'll check it in my envrionment later, but I have some urgent bugs, so, I cannot work on this soon, sorry.
Comment 5•7 years ago
|
||
Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3ae3c45db961&tochange=0a7ace2ffb9b Regressed by: 6f16a41f2318 Masayuki Nakano — Bug 1187579 Enable TSF in e10s mode r=m_kato When intl.tsf.enable=false (and restart browser), it fixes the problem with/without e10s. And it triggered to TSF on in Bug 478029 non-e10s.
Status: UNCONFIRMED → NEW
status-firefox51:
--- → wontfix
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox-esr45:
--- → wontfix
Component: Untriaged → Widget: Win32
Ever confirmed: true
Keywords: regression
Version: Trunk → 41 Branch
Updated•7 years ago
|
Summary: new line is ignored when use ATOK's template input function → [TSF] new line is ignored when use ATOK's template input function
Assignee | ||
Comment 6•7 years ago
|
||
Alice-san: You don't need to (and shouldn't) mark like this bug as a blocker of bug 478029 because it just enabled the pref. So, it doesn't point actual regression cause and it spreads bug spam to the watchers. Only adding "[TSF]" into the summary is enough.
Flags: needinfo?(ayakawa.m)
Assignee | ||
Comment 7•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d4fd3456fdd
Assignee | ||
Comment 8•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=59ad76119e05
Updated•7 years ago
|
Assignee: nobody → masayuki
Priority: -- → P2
Whiteboard: tpi:+
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8847977 [details] Bug 1339331 TextEventDispatcher should replace \r in composition string with \n and TextComposition should allow to input \n with composition events https://reviewboard.mozilla.org/r/120934/#review123250
Attachment #8847977 -
Flags: review?(m_kato) → review+
Comment 11•7 years ago
|
||
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/8e72178c3893 TextEventDispatcher should replace \r in composition string with \n and TextComposition should allow to input \n with composition events r=m_kato
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8e72178c3893
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 13•7 years ago
|
||
Should we consider this for uplift?
Assignee | ||
Comment 14•7 years ago
|
||
Honestly, I have no idea whether this should be uplifted or not. If we'll uplift this, the patches for bug 1347433 are also necessary to be uplifted. Fortunately, they and this have automated tests. So, the risk must not be so high. On the other hand, we've never gotten this report since TSF is enabled. So, I'm not sure how many users will be helped by the uplift. I know, initial TSF implementation already handled \r\n in composition string. IIRC, that was for tablet PC released with WinXP. However, we've already stopped supporting TSF on WinXP due to unstable and if such users still use Firefox on such devices, we must have gotten this reports earlier. So, it seems that only a few users have trouble with this bug but they don't think that this is not so inconvenience because nobody hasn't reported this. Makoto-san, do you have some ideas about this?
Flags: needinfo?(masayuki) → needinfo?(m_kato)
Comment 15•7 years ago
|
||
humm, change isn't small, so it is too late to uplift to beta. And we have a workaround not to use TSF. If change isn't big, we should uplift this to aurora. And, if critical, some people already alerts this regression after changing to TSF. But no reported... So I think that we shouldn't uplift to aurora, too.
Flags: needinfo?(m_kato)
Assignee | ||
Comment 16•7 years ago
|
||
Thanks, let's give up to fix this on any branches.
Assignee | ||
Comment 17•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b977966b7ae5e49c47db5506641d75883284e58
Assignee | ||
Comment 18•7 years ago
|
||
This should be uplifted for fixing bug 1358958 which is critical for Korean IME users on macOS.
Assignee | ||
Comment 19•7 years ago
|
||
Comment on attachment 8847977 [details] Bug 1339331 TextEventDispatcher should replace \r in composition string with \n and TextComposition should allow to input \n with composition events [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is required to fix bug 1358958 which is really annoying bug for Korean IME users on macOS. User impact if declined: A few ATOK (Japanese IME) users cannot use its template function if template includes line breakers. Korean IME users of macOS needs to type Enter key twice to insert a line break during inputting Hangul characters. Fix Landed on Version: 55 Risk to taking this patch (and alternatives if risky): Not risky because this makes TextComposition which manages IME composition in content allow to include \n in composition/commit string. String or UUID changes made by this patch: No. Approval Request Comment [Feature/Bug causing the regression]: Perhaps, regression of bug 1158456. [User impact if declined]: See above. This is required to fix bug 1358958. [Is this code covered by automated tests?]: Yes. The patch includes automated tests. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: ATOK isn't free to charge and it has only Japanese UI to setup and use. So, it's difficult to check the original report's symptom. However, after landing this and the patch for bug 1358958, you can check this with manual QA with Korean IME. 1. Focus to a <textarea>, e.g., in bugzilla. 2. Activate 2-Set Korean IME on macOS. 3. Type 'a' and type 'Enter'. Then, it should cause breaking the line. [List of other uplifts needed for the feature/fix]: No. [Is the change risky?]: No. [Why is the change risky/not risky?]: See above. And this patch has automated tests. [String changes made/needed]: No.
Attachment #8847977 -
Flags: approval-mozilla-esr52?
Attachment #8847977 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 20•7 years ago
|
||
Comment on attachment 8847977 [details] Bug 1339331 TextEventDispatcher should replace \r in composition string with \n and TextComposition should allow to input \n with composition events Sorry for the spam. This patch cannot graft from mozilla-central.
Attachment #8847977 -
Flags: approval-mozilla-esr52?
Attachment #8847977 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 21•7 years ago
|
||
Sigh, this cannot be uplifted without patches for bug 1347433. And they shouldn't be uplifted due to the risk management...
Comment 22•7 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) (PTO: 5/26) from comment #21) > Sigh, this cannot be uplifted without patches for bug 1347433. And they > shouldn't be uplifted due to the risk management... What do you think is so risky about the patches from that bug? Are you worried about webcompat issues?
Flags: needinfo?(masayuki)
Comment 23•7 years ago
|
||
Like :masayuki mentioned in comment #21, it's too risky given we are in the very late Beta54 cycle. I prefer to let this ride the train and won't fix in 54.
Assignee | ||
Comment 24•7 years ago
|
||
Not the webcompat issue. However, basically, like the patches for bug 1347433 which redesign the initializer of events before dispatching shouldn't be uplifted to anything, especially, ESR.
Flags: needinfo?(masayuki)
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•