Last Comment Bug 711648 - Pre-commit underline is not shown for the composing text
: Pre-commit underline is not shown for the composing text
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P4 normal (vote)
: Firefox 12
Assigned To: Alex Pakhotin (:alexp)
:
: Sebastian Kaspari (:sebastian)
Mentors:
http://people.mozilla.com/~nhirata/ht...
Depends on: 743468
Blocks: 720333
  Show dependency treegraph
 
Reported: 2011-12-16 15:37 PST by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2012-04-23 13:44 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
WIP Patch (12.55 KB, patch)
2012-01-13 23:35 PST, Alex Pakhotin (:alexp)
no flags Details | Diff | Splinter Review
Patch 2 (11.55 KB, patch)
2012-01-17 23:33 PST, Alex Pakhotin (:alexp)
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
WIP Patch 3 (17.03 KB, patch)
2012-01-19 18:48 PST, Alex Pakhotin (:alexp)
no flags Details | Diff | Splinter Review

Description Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-12-16 15:37:51 PST
1. go to http://people.mozilla.com/~nhirata/html_tp/formsninput.html
2. long tap in a text field and select simeji
3. click in a text field
4. type some japanese characters

Expected: underline underneath the Japanese character before committing
Actual: no underline.

used : fennec-11.0a1.en-US.android-arm_1324041795
on Samsung Galaxy S II Android OS 2.3
Comment 1 Alex Pakhotin (:alexp) 2011-12-16 16:26:32 PST
With the bug 595008 fix the underline for the composing text is not displayed now regardless of the IME used. That's due to different handling of the IME events.
It is a step back compared to the previous implementation, but still on par with the stock browser, which does not show the underline either.
I am planning to look into this, but I believe it's not a high priority issue.
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-12 09:40:34 PST
We probably need this for CJK IMEs
Comment 3 Alex Pakhotin (:alexp) 2012-01-13 23:35:04 PST
Created attachment 588606 [details] [diff] [review]
WIP Patch

Implements the composition style handling. The patch needs a bit tweaking though to make sure everything works in all situations.
Comment 4 Alex Pakhotin (:alexp) 2012-01-17 23:33:28 PST
Created attachment 589415 [details] [diff] [review]
Patch 2

Composition events are now issued more correctly, with the composing transaction kept open while the text is being edited.

setComposingRegion() still needs a better implementation.
Comment 5 Alex Pakhotin (:alexp) 2012-01-19 18:48:45 PST
Created attachment 590079 [details] [diff] [review]
WIP Patch 3

Added handling for setComposingRegion(), but this is really work-in-progress, which needs to be fixed, as it causes very weird behavior with SwiftKey X.
Comment 6 Alex Pakhotin (:alexp) 2012-01-19 18:59:28 PST
Comment on attachment 589415 [details] [diff] [review]
Patch 2

The second version of the patch is probably the best way to go at the moment. It implements the most of the functionality without major side-effects.

The composition events are issued more or less correctly, the composing text style is handled in most cases, with pre-commit underline being displayed.

Later if needed I may have another approach to further improve this in a follow-up bug.
Comment 7 Ed Morley [:emorley] 2012-01-21 07:10:37 PST
https://hg.mozilla.org/mozilla-central/rev/c53d7932f6b7
Comment 8 Alex Pakhotin (:alexp) 2012-01-24 10:41:14 PST
Comment on attachment 589415 [details] [diff] [review]
Patch 2

[Approval Request Comment]
Regression caused by (bug #): Bug 595008
User impact if declined: IME composition events are fired not quite correctly; Slower typing with VKB with extra selections of the text; No underline and special text style while typing with VKB, which is important especially for Japanese and similar languages.
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): Possible regressions for untested IMEs.
Comment 9 Alex Keybl [:akeybl] 2012-01-25 16:59:45 PST
Comment on attachment 589415 [details] [diff] [review]
Patch 2

[Triage Comment]
Mobile only - approved for Aurora.
Comment 10 Brad Lassey [:blassey] (use needinfo?) 2012-01-30 11:45:19 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/fa0ab73ef86f

Note You need to log in before you can comment on or make changes to this bug.