Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On input for Itext #2352

Merged
merged 3 commits into from
Jul 22, 2015
Merged

On input for Itext #2352

merged 3 commits into from
Jul 22, 2015

Conversation

asturur
Copy link
Member

@asturur asturur commented Jul 20, 2015

closes #2051
closes #2318
closes #1747

@sapics could you try to enter some japanese text?
@marcospassos can you try it
@angelim @taveras

demo here:
http://www.deltalink.it/andreab/fabric/kitchensink.html

@sapics
Copy link
Contributor

sapics commented Jul 21, 2015

@asturur Great job!

I test it in windows8.1 chrome 64bit.
It looks work well for one japanese-text.
But, when I input あお (two japanese text) in japanese, only お is displayed.

@asturur
Copy link
Member Author

asturur commented Jul 21, 2015

i got it i have to compare text lengths.
also from android in input words with swipe method i m getting only first word.

i think same for japanese

@asturur
Copy link
Member Author

asturur commented Jul 21, 2015

@sapics multiple chars should be ok now.
But i have a race condition for copy - paste that is triggering the onInput event, i will fix that later

@asturur
Copy link
Member Author

asturur commented Jul 21, 2015

@kangax ready for review.

textLength = this.text.length,
newTextLength = this.hiddenTextarea.value.length,
diff = newTextLength - textLength,
insChar = this.hiddenTextarea.value.slice(cp, cp + diff);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insCharcharsToInsert

@asturur
Copy link
Member Author

asturur commented Jul 21, 2015

@kangax changes applied

@kangax
Copy link
Member

kangax commented Jul 21, 2015

LGTM, just need a confirmation that it works! /cc @sapics

@sapics
Copy link
Contributor

sapics commented Jul 22, 2015

First of all, sorry my bad english. :)

Now, it works about あお!

However, when i want to input あか, it inputs as あkあ.
keyboard ⇒ Japanese
a ⇒ あ
ka ⇒ か
o ⇒ お

And, transformation Hiragana to Kanji looks not work.
For example, by pressing SPACE after input あか
あか ⇒ 赤 ⇒ 亜科 ⇒ 垢
we change hiragana to kanji.

default

@asturur
Copy link
Member Author

asturur commented Jul 22, 2015

@sapics if i use input method japanese, do you think i can emulate your settings/situation?

we did a step further anyway is lot better than before.

@sapics
Copy link
Contributor

sapics commented Jul 22, 2015

@asturur I think that yes you can!

@asturur
Copy link
Member Author

asturur commented Jul 22, 2015

@sapics your example is perfect.
i have to trash it and do differently.

@asturur
Copy link
Member Author

asturur commented Jul 22, 2015

@sapics but do you expect Right to Left writing when using the japanese input?
Because as of now, i do not understand how it should work.
I see that i can do:
a ka ( becomese the symbol 'ka' ) no (become symbol 'no' ) then i press space, and a + ka becomes a new symbol. but 'no' get untouched.

How this transformation works?
it transform all line symbol or just the last input?

@asturur
Copy link
Member Author

asturur commented Jul 22, 2015

@sapics http://www.deltalink.it/andreab/fabric/kitchensink.html
check if it feels natural.
Consider that deleting and inserting newline will freeze browser....
Tell me if the input is behaving as you would expect.
There are selection and cursor glitches.

@kangax i would pull as it is JUST for accent support and give JP and CHN input a later review.
Is gonna be quite a lot of work.

@asturur
Copy link
Member Author

asturur commented Jul 22, 2015

image

kangax added a commit that referenced this pull request Jul 22, 2015
@kangax kangax merged commit 989d1fa into fabricjs:master Jul 22, 2015
@asturur asturur deleted the onInput branch July 22, 2015 12:52
@sapics
Copy link
Contributor

sapics commented Jul 22, 2015

@asturur Great Job! Now, Japanese input works!

By the way, cursor position and input place looks strange.
ezgif com-optimize

@asturur
Copy link
Member Author

asturur commented Jul 23, 2015

the important thing is that it looks right.
it require some work to be completely supported.

@jcppman
Copy link

jcppman commented Jul 23, 2015

@asturur @kangax

I'm afraid that this solution (detecting length change) is not robust enough, cuz for accent-based input methods, there's no relation between length changes and content changes, for example sometime's when user type one character, length might be still the same but contents inside it has changed, or with some input behavior the length will become shorter. In a word, charsToInsert can't rely on length changes.

Accent-based characters composing procedure has inner-states, the ultimate solution is this: https://developer.mozilla.org/en-US/docs/Web/API/CompositionEvent but it is not stable enough yet, however, currently there's something we can rely on: selection. The characters that are still in compositing will alwasy be selected, so what we need to do is just replace the contents in current selection of IText with the contents in current selection of hiddenTextarea.

Another issue is this.insertChars will update hiddenTextarea, both it's content and it's selection, which will break some accent-based methods' inner-state and interupt the input procedure.

Actually if you have time, you may want to take a look at #2243, I've been through all those pitfalls when I implement this patch, includes the bug @sapics mentioned. Something might not make sense to you but I did all those tweaks for reasons. It's totally fine if you don't like my code and prefer to implement it from scratch, I'm just saying that don't fall into the same pitfalls again if it is avoidable. We've tested this patch on:

  • Windows 7, Chrome
  • Windows 7, Firefox
  • Mac OSX 10.10 Chrome
  • Mac OSX 10.10 Firefox
  • iOS cordova app
  • iOS Chrome

it works and especially works well with #1990 together cuz the options box will be placed properly. So I guess it is not totally useless.

@asturur
Copy link
Member Author

asturur commented Jul 23, 2015

can you show some accent that are not gonna work with this solution?

we totally understand that this solution does not work with the jpn and chn languages, for wich we planned a later review.

i pushed it later because as of now best solution is to invert the text source.

this.text = this.hiddenTextarea.value

but requires too many changes now.
also the position of selectionbox is something will be fixed with textarea positioning later

i also replied to @sapics to his example a ka no + space "i have to trash this and do it differently".
that example put in evidence all the weakness of this solution and also that once we invert the text source it starts to work properly.

@ajck
Copy link

ajck commented Aug 5, 2015

I just tried Fabric rc-1.6.0 with an IText, using latest Chrome browser on Win 7. I installed Hebrew, Arabic, Hindi and Russian keyboard layouts on Win 7 (built in ones, not 3rd party app).

I found typing characters in languages which have multi byte characters/modifiers always adds an extra space on the end of the line after the actual characters you want, on the right hand side and also places the cursor there, but the typed characters get added in the right place. So this happens with Hindi for example which is written Left To Right. but not Russian as that is one character per key press . However with right-to-left languages such as Arabic and Hebrew, the typed characters get added simultaneously on the left hand side of the whole line whilst cursor and extra spaces still appear on the left (thus cursor and text entry point get further apart with each new character).

Not sure if fix is planned for 1.6.0? Would be amazing to offer full international text entry!

Many thanks for great work @kangax and @asturur !

@asturur
Copy link
Member Author

asturur commented Aug 6, 2015

We need to rework quite lot of things to get full onInput support.
And as of now, because we spent lot of hours on iText lately, we would like to fix also other things in the library. For sure we will finish it, we are so close.

@astw
Copy link

astw commented Sep 11, 2015

+1 for Chinese input.
Thanks @asturur

@nakamorichi
Copy link

Here seems to be a solution (http://jsfiddle.net/3jk3jvy7/light/), but I'm not sure how to apply it to Fabric.js. I'm using Fabric.js as part of Pixie. Could someone tell me how to apply the patch?

@asturur
Copy link
Member Author

asturur commented Sep 18, 2015

you should download the attacched fabric.min.js, deminimize it, port the changes from 1.4.0 to 1.6.0

We had correctly working multibyte demo, but it was lacking style.
I should finish it but we have so many things to do now, i really do not have time.
But this has been considered, sort of solved, but stopped for lack of support for existent features.

@nakamorichi
Copy link

@asturur could you share the fabric.js file where you had it working? I don't care about text styles, but it is very important that multi-byte text input works.

@nakamorichi
Copy link

I applied the patch provided by @jcppman (#2243), and it seems to work perfectly (at least for Japanese). Why isn't the patch yet merged into main branch, considering that Japanese and Chinese are very significant language groups to support?

@asturur
Copy link
Member Author

asturur commented Sep 24, 2015

because that patch has mixed input scenario with oninput and onkeypress. while we want to switch to oninput directly but we need to setup styles first and cursor position.

@0x1ad2
Copy link

0x1ad2 commented Dec 23, 2015

The Android keyboard is still not pop-ing up when editing a iText object.
I'm running Android 5.0.1

  When I removed obj.enterEditing(); it did work!
  So somehow the phone doesn't know to pop-up the
  keyboard by default when enterEditing is applied.

@angelim
Copy link

angelim commented Mar 27, 2016

Hi @asturur,
I apologize for not seeing you mentioned me to give feedback. I don't know what happened with that notification.

On Mac 10.9.5 this is what I get when using the kitchensink demo on chrome 49, Firefox 44 and Safari 9.0.1:
Chrome: Accents are placed before characters: `a´e

Firefox: same as above

Safari: Accents are placed twice; before and over the character: `à, ´é.

Using the vanilla text input right bellow the canvas works as expected.
@Kitanotori's jsfiddle based on @jcppman patch works as well.

@asturur
Copy link
Member Author

asturur commented Mar 28, 2016

We changed quite a bit of things on text input lately ( we switched to oninput event) did you try latest version from thw github repository?

@ajck
Copy link

ajck commented Mar 28, 2016

@asturur - great to hear text input developments - what's the main benefits/effects of these changes (e.g. oninput event). Also, how to get the latest version - where on these github pages is the right link please? (I don't use git). Many thanks

@asturur
Copy link
Member Author

asturur commented Mar 28, 2016

Mainly accented letter should work evey os/language.
Some chines and japanes input is working.
Better copy paste.

I need to make an auto builder it would help many people....

@ajck
Copy link

ajck commented Mar 28, 2016

@asturur thanks - how can I download latest version please, where is link on the github (i.e. this) site?

@angelim
Copy link

angelim commented Mar 28, 2016

I've just cloned fabricjs repository, built from source using node build.js modules=ALL and replicated the kitchensink example locally pointing to the generated fabric.js. Same results.
kitchensink

@asturur
Copy link
Member Author

asturur commented Mar 28, 2016

can you share OS, browser, keyboard and input method?
I'm sorry that there is always some hardware/software combination that screws it

@angelim
Copy link

angelim commented Mar 28, 2016

Mac Retina Mid 2012 with OSX 10.9.5 on chrome 49.0, Firefox 44 and Safari 9.0.1. The keyboard layout is US International. Also tried on a Mac OSX 10.11.13, with the same results.
Chrome: Accents are placed before characters: `a´e (as per the previous screenshot)

Firefox: same as above

Safari: Accents are placed twice; before and over the character: `à, ´é. (this is the oddest one. One input generates two chars).

This only happens if I try to edit directly at the canvas. If I use the execute function or use the vanilla text input, it renders perfectly.

@asturur
Copy link
Member Author

asturur commented Mar 28, 2016

added to temp test page brilliant fix from #2820.

http://www.deltalink.it/andreab/fabric/iphone.html

Can you @angelim check if now accent works as expected?

@angelim
Copy link

angelim commented Mar 28, 2016

YES!!!!!!

@nakamorichi
Copy link

@asturur Copy from IText still not working in IE(11) and Edge.

@nakamorichi
Copy link

Also still practically impossible to write Japanese in IText.

@raffareis
Copy link

@asturur When using version 1.6.0-rc1 i had the exact same issue as @angelim (using Mac OS, keyboard Brazilian - Pro):

Chrome: Accents are placed before characters: `a´e (as per the previous screenshot)

Firefox: same as above

Safari: Accents are placed twice; before and over the character: `à, ´é. (this is the oddest one. One input generates two chars).

Then i tried your demo (http://www.deltalink.it/andreab/fabric/iphone.html) which solved the issue for @angelim, but in my case, both the demo and releases 1.6.0+, the new behavior is that when typing +a**, i get the ** but when pressing a, nothing happens (it stays **). by pressing **a** again (second time) i get **a (by typing `+a+a)

all three strokes cause an onkeydown event, key codes: (backQuote, KeyA,KeyA)

it works ok on any textarea or input, it doesn't work on iText

@asturur
Copy link
Member Author

asturur commented May 6, 2016

i noticed this regression on mac last week. o have to sit on a mac and check. any mac browser?

@raffareis
Copy link

raffareis commented May 6, 2016

This time, both safari and chrome have the exact same behavior.

Tested on windows, and it works perfectly as it should.

If there is anything i can do to help, like attaching prints of some console debugging, let me know

@raffareis
Copy link

@asturur can you reopen this issue?

@asturur
Copy link
Member Author

asturur commented May 16, 2016

this was a pr, i cannot open again. Can you describe me the actual issue? what is not working with wich keyboard and operaring system browser. i m getting an apple notebook if it may help.

@raffareis
Copy link

Version 1.6.0-rc1

When using version 1.6.0-rc1 i had the exact same issue as @angelim (using Mac OS, keyboard Brazilian - Pro, but i tested in many configurations that accept accents - spanish, italian, etc), as follows:
Chrome: Accents are placed before characters: ´a´e (as per the previous screenshot)
Firefox: same as above
Safari: Accents are placed twice; before and over the character: `à, ´é. (this is the oddest one. One input generates two chars).

Version 1.6.1+

Then i tried your demo (http://www.deltalink.it/andreab/fabric/iphone.html) which solved the issue for @angelim, but in my case, both the demo and releases 1.6.0+, the new behavior is that when typing ´+a, i get the ´ but when pressing a, nothing happens (it stays ´). by pressing a again (second time) i get ´a (by typing ´+a+a, in total)

  • it happens with any combination of accents and letters, not only ´ and a
  • all three strokes cause an onkeydown event, key codes: (backQuote, KeyA,KeyA)
  • it works ok on any common html textarea or input, it doesn't work on iText
  • it works perfectly on windows.

@asturur
Copy link
Member Author

asturur commented May 16, 2016

as soon as i have a mac, i will put myself on this.

@ajck
Copy link

ajck commented May 16, 2016

Thanks @asturur . I think one of the main issues and hopefully easiest to solve is when any multi-character alphabets are used, e.g. Arabic and many others, for each visual character that is typed or copy-pasted, an extra space appears so an itext starts looking like "ص|" (where | is the cursor) and as you type it is "صقفغ___ |" and then "صقفغغعارةتوخح________ |" (I've used _ underscore for a space here as this editor seems to delete them) with no way to remove that space. (on the subject of Arabic - and Hebrew etc - would be cool to be able to enter Right To Left (RTL) text, but that's a separate issue I guess!).

Thanks,

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
10 participants