670837 - maxlength shouldn't count one non-BMP character as two characters

Status: NEW

(Core :: DOM: Editor, defect)

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:
1. Enter data:text/html,<input maxlength=3> into the location bar.
2. Type (or copy and paste) 

Comment 1

[Masatoshi Kimura [:emk]]

Oops, Bugzilla didn't accept non-BMP characters...

Steps to reproduce:
1. Enter data:text/html,<input maxlength=3> into the location bar.
2. Type (or copy and paste) &#x20BB7;野家 into the text box.
Expected result:
&#x20BB7;野家
Actual result:
&#x20BB7;野

Comment 2

[Masatoshi Kimura [:emk]]

maxlength is a maximum allowed value length in code-point (not UTF-16 code unit) per HTML5 spec.
Chrome works as expected.

Comment 3

[Boris Zbarsky [:bzbarsky]]

For what it's worth, WebKit's maxlength impl counts glyph clusters.  We should do the same, but we don't have good infrastructure for it....  We probably need a bug on said infrastructure, and also some conversation between Mounir and whoever implements it on how it needs to work for performance to not be hosed.

Comment 4

[Boris Zbarsky [:bzbarsky]]

And I think the HTML5 spec is sort of wrong here.  Maxlength should not count combining marks and the like, imo.

Comment 5

[Simon Montagu :smontagu]

That depends why you want maxlength in the first place. See http://lists.w3.org/Archives/Public/www-international/2011AprJun/0119.html

A more serious case than the STR in comment 1 is entering 野家𠮷 -- this leaves an unpaired surrogate in the input field.

Comment 6

[Mounir Lamouri (:mounir)]

The DOM doesn't do anything with maxlength except the attribute reflection, even validation has been disabled before shipping Firefox 4. This should be fixed in the editor.
FWIW, we might change the behavior with maxlength to not prevent typing but just making the field invalid if the text length is greater than maxlength, see bug 613016.

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to comment #3)
> For what it's worth, WebKit's maxlength impl counts glyph clusters.  We
> should do the same....

I don't think I agree with this, in general. For cases like Latin letters with accents, it seems a reasonable interpretation (although as Simon mentions, this depends on the use case); however, for Indic scripts where a "cluster" may consist of multiple conjoined consonants, plus a vowel mark, plus additional marks such as nasalization, I don't believe it makes any sense (to users) to count the "length" of a string in terms of glyph clusters; they are well aware of the constituent characters within such clusters and would expect to count them separately.

Whether "length" (for the purposes of maxlength) is most usefully measured in terms of Unicode characters or UTF16 code units is a tricky question, but given that Javascript and DOMStrings expose the UTF16 encoding form, I think it would be most consistent for maxlength to be expressed as a count of UTF16 units, too.

Comment 8

[Simon Montagu :smontagu]

(In reply to comment #7)

> Whether "length" (for the purposes of maxlength) is most usefully measured
> in terms of Unicode characters or UTF16 code units is a tricky question, but
> given that Javascript and DOMStrings expose the UTF16 encoding form, I think
> it would be most consistent for maxlength to be expressed as a count of
> UTF16 units, too.

Either way, we shouldn't truncate in mid-supplementary character. If maxlength is interpreted as UTF-16 code units, "野家𠮷" with a maxlength of 3 should give "野家"

Comment 9

[Jonathan Kew [:jfkthame]]

(In reply to comment #8)
> Either way, we shouldn't truncate in mid-supplementary character. If
> maxlength is interpreted as UTF-16 code units, "野家𠮷" with a
> maxlength of 3 should give "野家"

Definitely. We should never allow unpaired surrogates.

(Well, I don't think we can prevent people creating them via JS string-munging. So our code needs to handle them robustly anywhere they occur. But wherever possible, we should prevent them arising.)

Comment 10

[Boris Zbarsky [:bzbarsky]]

It really sounds like there are two separate issues here:

1)  Fix editor to not truncate in mid-surrogate-pair.
2)  Spec discussion that needs to happen (and I'm probably the wrong person to
    drive it).

Who's willing to take on #2?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Note that #2 is happening in https://github.com/whatwg/html/issues/1467

Comment 12

[Martijn]

I have created a pen that makes it slightly easy to check:

https://codepen.io/thany/pen/zmRZKM

My current findings:
Firefox 64 has this bug
Chrome 72 has this bug
Edge 17 does NOT have this bug

Comment 13

[Thomas Wisniewski [:twisniewski]]

Would fixing this align all browsers on this WPT? https://wpt.fyi/results/html/semantics/forms/constraints/input-maxlength-emoji.html

Comment 14

[Jonathan Kew [:jfkthame]]

No; it looks like that WPT case fails because of its use of execCommand("InsertHTML", ...), which Firefox doesn't seem to support in an <input type=text> (quite logically, IMO).

Changing the test to use execCommand("InsertText", ...) makes it pass, AFAICS from trying locally.

Comment 15

[Thomas Wisniewski [:twisniewski]]

Oh, interesting.. I don't think there are any WPTs to cover that. Perhaps masayuki knows better..

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

Well, I think that the test should be rewritten with insertText because there is no agreements about serialization from HTML fragment to plaintext value in <input> and <textarea>.

Comment 17

[Thomas Wisniewski [:twisniewski]]

I don't disagree, but that still will leave us with the interop problem with insertHTML, won't it? Should we file any issues against the spec/WPTs/etc?

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

Yeah, I think so. The problem about insertHTML should be discussed separately from this bug.