1066756 - double click word selection expansion stopping at u+A789 (ModifierSymbol : "Sk")

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P4)

Description

[Tom Hindle]

Attachment: UA789.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0
Build ID: 20131028113246

Steps to reproduce:

1. Load Attached sample document
which contains:
hello꞉world 
where ꞉ is u+A789
2. double click on hello or world.


Actual results:

only hello or world is selected.


Expected results:

the whole word "hello꞉world" should be selected.

IE selects the entire string.
Chrome behaves like firefox. (which I beleive to be the incorrect behaviour)

Comment 1

[Mats Palmgren (inactive)]

It looks like this is intentional - we stop at kPunctuation or kSymbol:
http://hg.mozilla.org/mozilla-central/annotate/d070787de8f7/layout/generic/nsTextFrame.cpp#l6726

u+A789 belongs to the Unicode "Symbol, Modifier [Sk]" category:
http://www.fileformat.info/info/unicode/char/a789/index.htm

and we map that to kSymbol here:
http://hg.mozilla.org/mozilla-central/annotate/d070787de8f7/intl/unicharutil/util/nsUnicodeProperties.cpp#l91

Comment 2

[Mats Palmgren (inactive)]

The list of characters in the 'Symbol, Modifier' category:
http://www.fileformat.info/info/unicode/category/Sk/list.htm

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: include characters with Unicode category Sk (modifier symbol) in double-click word selection.

Looking at the collection of 'Sk' characters, I think it'd make sense in general to include them as part of the 'word' for double-click selection purposes. Excluding them from the chars recognized by ClusterIterator::IsPunctuation() should fix this.

Comment 4

[Jonathan Kew [:jfkthame]]

If we agree this change makes sense, we should also add suitable testcases to layout/generic/test/test_movement_by_words.html.

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 8489097 [details] [diff] [review]
include characters with Unicode category Sk (modifier symbol) in double-click word selection.

Seems reasonable, r=mats

Nit:
Perhaps you'd want to capitalize the commit message:
bug 1066756 - include ...
^             ^
to follow common practice.

Also, there's a minor error in the code comment here:
http://hg.mozilla.org/mozilla-central/annotate/d070787de8f7/intl/unicharutil/util/nsUnicodeProperties.cpp#l93
The hb_unicode_general_category_t enum now lives in
gfx/harfbuzz/src/hb-unicode.h

And don't forget to add the tests you suggested, thanks!

Comment 6

[Jonathan Kew [:jfkthame]]

Tidied up nits; added a test using the MODIFIER LETTER COLON, as in the original testcase here.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e1129bb236

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a8e1129bb236