Setting a ui::CompositionUnderline in　setMarkedText:selectedRange:replacementRange: in BridgedContentView (https://cs.chromium.org/chromium/src/ui/views/cocoa/bridged_content_view.mm?q=bridged_con&sq=package:chromium&l=1209) seems to work. Any reason we are not already doing this?

no reason - let's do it! (i.e. whatever gets us most consistent with {content area, NSTextField, other users of ui::CompositionText})

A change landed for this - https://codereview.chromium.org/2218773002/. Not sure why the bot did not refer it here. The composition text should now be underlined.

Pending changes (Render Text implementation does not currently support these)-
1) Target clause should have a thick underline (instead of being selected).
2) Discontinuous underlines for different clauses.

tapted@: we're using RenderTextHarfBuzz on Mac now, right? Does that mean the missing features in #4 are now present for us? If so I think we can close this.

I am not sure if we are using RTHB on Mac by default now. Even if we are, the things mentioned in c#4 would still need implementation. RenderText itself (and hence RTHB as well) did not support thick and discontinuous underlines.

*discontinuous underlines for consecutive characters.

See Issue 769055 for another effect

This bug is still live and should be fixed before we ship MacViews.

patti, can you take a peek? :)

These Proj=MacViews bugs should probably be tracking for Phase 3 ( Issue 603386 ) - m66.

I just tested this and this bug seems to be now Fixed. tapted@, does that line up with your understanding? I'm surprised that it appears Fixed, since there aren't any changes (that I can see) to support it, but I definitely see the expected composition underline.

If this is indeed Fixed, please close it - otherwise please kick it back to me for triage :) Thanks!

There were still two parts:

1) Target clause should have a thick underline (instead of being selected).
2) Discontinuous underlines for different clauses.

RenderText's formatting APIs currently only support one underline style. But it should be straightforward to add "thick underline" and "skip whitespace" styles.

Okay, I'll work on this.

Heavy underlines for composition: <https://chromium-review.googlesource.com/c/chromium/src/+/905125>

There is an odd discrepancy I've noticed between Views textfields and Cocoa textfields.

In a Cocoa text field kv<enter>mq has the text field contain: kv<u>mq</u>.
In a Views text field, kv<enter> works right, but then when you hit m, you get: kv<u>&space;m</u>, and then when you type q, you get no output at all, and hitting enter leaves the field containing kv&space;m

So, there's an extra space getting inserted somewhere, and maybe some off-by-one thing going on. I'll take a peek.

I had a look at the behavior with the spaces. It happens with Katakana, but *not* with Hiragana, and the extra space is coming from NSTextInputContext itself - i.e., it passes the string with the extra space into [BridgedContentView setMarkedText:selectedRange:replacementRange], which is weird. I'm starting to think if this is an AppKit bug.

I tried Katakana input in Dash (which is using a vanilla Cocoa text field) and it displays the same issue. The Safari omnibox doesn't, which makes me think Safari has a workaround in place for this. Anyway, I'm not mega-worried about fixing it.

tapted or karandeepb: do either of you know how to trigger a composition that should have multiple discontinuous underlines?

Here's an example: With Katakana, press 'a' 8 times. Press the right arrow key, this allows you to select different clauses, with the currently active clause having a thick underline.

#21: that didn't work for me, but I think I got it to happen in TextEdit:

1) In Katakana, type "nihon".
2) Down arrow twice, to get a completion whose text is "ニホn"
3) See the heavy underline underneath the "ニホ" and the lighter underline underneath the "n".

Passing the underline state from BridgedContentView to TextInputClient: <https://chromium-review.googlesource.com/c/chromium/src/+/919183>

Supporting heavy underlines in RenderText and using them for composition ranges: <https://chromium-review.googlesource.com/c/chromium/src/+/905125>

The missing link is to take the composition ranges in TextfieldModel and inform RenderText about them somehow - probably we need RenderText::SetTargetClauseRange() in addition to RenderText::SetCompositionRange(), and we need TextfieldModel::SetCompositionText() to call it.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa33391487b985f547b77672d5ab5e9cc2a95534

commit fa33391487b985f547b77672d5ab5e9cc2a95534
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Feb 14 18:56:26 2018

gfx: use heavy underline for compositions in RenderText

This change:
1) Adds a HEAVY_UNDERLINE text style
2) Has RenderText apply it for compositions, instead of UNDERLINE
3) Has RenderTextHarfBuzz and RenderTextMac draw HEAVY_UNDERLINE properly

Bug: 612675
Change-Id: I08f997dfb1edee1ea29d8c828302d7a24418975b
Reviewed-on: https://chromium-review.googlesource.com/905125
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536765}
[modify] https://crrev.com/fa33391487b985f547b77672d5ab5e9cc2a95534/ui/gfx/render_text.cc
[modify] https://crrev.com/fa33391487b985f547b77672d5ab5e9cc2a95534/ui/gfx/render_text.h
[modify] https://crrev.com/fa33391487b985f547b77672d5ab5e9cc2a95534/ui/gfx/render_text_harfbuzz.cc
[modify] https://crrev.com/fa33391487b985f547b77672d5ab5e9cc2a95534/ui/gfx/render_text_harfbuzz.h
[modify] https://crrev.com/fa33391487b985f547b77672d5ab5e9cc2a95534/ui/gfx/render_text_mac.mm
[modify] https://crrev.com/fa33391487b985f547b77672d5ab5e9cc2a95534/ui/gfx/text_constants.h

Tried to verify the fix on 66.0.3348.0 using Mac 10.13.3 and didn't observe any change in builds before fix and after fix. 

1. From Input sources of system preferences added japanese.
2. Selected Katakana and as per comment#21, inputed a 8 times and pressed right arrow -- Observed thick underline both in 66.0.3346.0 and 66.0.3348.0
3. Selected Katakana and as per comment#22, inputed nihona and pressed down arrow twice now seeing ニホン instead of ニホn (as mentioned in comment#22) -- Observed thick underline both in 66.0.3346.0 and 66.0.3348.0 and didn't observe ニホn.

@ellyjones: Could you please check the screencasts and let us know if we miss anything from steps and please help in verifying the fix.

Deleted: 612675 in 66.0.3346.0.mp4 3.1 MB

	612675 in 66.0.3346.0.mp4 3.1 MB View Download

Deleted: 612675 in 68.0.3348.0.mp4 4.3 MB

	612675 in 68.0.3348.0.mp4 4.3 MB View Download

#25: there are two other CLs in flight that have not yet landed, so please hold off on testing for now :)

Mac triage: targeting this for M67.

As this is targeted for M67, pls have fix landed ASAP to trunk. Thank you

** Bulk Edit **

There are only two M67 dev releases left on 04/03 & 04/10 before M67 branch on 04/12. Please try to land the fix ASAP to trunk so we can move forward with 50%-50% experiment on M67 Canary/Dev (if possible at all). Thank you.

** Bulk Edit **

There is only one dev release left 04/10 before M67 branch on 04/12. Please try to land the fix ASAP to trunk so we can move forward with 50%-50% experiment on M67 Canary/Dev (if possible at all). Thank you.

FYI: Change/Fix has to be landed in trunk latest by 4:00 PM PT, Friday (04/06) so we can pick it up for next week dev release. 

ellyjones@, when do we expect remaining two CLs to land in trunk per comment #26? 

It isn't likely that these will get landed by M67. They aren't critical though so I'm kicking this bug to M68.

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.