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

[IText] IText is unusable for accent based languages #2051

Closed
marcospassos opened this issue Mar 22, 2015 · 24 comments · Fixed by #2352
Closed

[IText] IText is unusable for accent based languages #2051

marcospassos opened this issue Mar 22, 2015 · 24 comments · Fixed by #2352
Labels

Comments

@marcospassos
Copy link

Currently, the IText input binding strategy has some issues and one of most critical one is that IText is completely unusable for accent based languages (such as Portuguese, Spanish, Japanese, Chinese, etc) due the lack of support to dead keys.

The problem
IText uses a hidden input for capturing user's input, through the keydown event. The problem is that keydown events originated from dead keys cannot be converted into characters, because the event does not provide the character's code but a fixed key code (229) instead.

Examples:

Key Key Code Expected Result
~ 229 ~
' + c 229, 229 ç
~ + a 229, 229 ã
" + u 229, 229 ü
~ + ~ 229, 229 ~~
~ + ~ + ~ + a 229, 229, 229, 229 ~~ã
~ + [space] 229, 229 ~ [trimmed, no space]

Notice that pressing a after a dead key produces another dead key event. So, it's very hard to handle it in the way it's currently architected.

Holding the key of a given character to choose one of its variants is OS specific and almost no user of accented keyboards is familiar with this, so it's not a good alternative IMHO. I've been trying to fix it in the last days, but seems like we should change the way it is currently done, once we'll have to reimplement basic browsers/OS features bringing a lot of unnecessary code overhead.

Unfortunately, we've been working on a big startup project in the last months without noticing this issue and maybe we'll have to stop it immediately, at least until discover a workaround.

Related issues: #1175 and #1954

@marcospassos marcospassos changed the title IText is unusable for accent based languages [IText] IText is unusable for accent based languages Mar 22, 2015
@asturur
Copy link
Member

asturur commented Mar 23, 2015

With american keyboard on windows everything is fine.
Is a matter of operating system.

There was a PR somewhere in wich usin onInput event was fixing everything for everyone.

@marcospassos
Copy link
Author

@asturur The PR #1954 does not work anymore on master.

And yes, IText is only 100% usable on Windows with american keyboard layout, which is a big limitation.

@asturur
Copy link
Member

asturur commented Mar 23, 2015

Of course is a big limitation, and will be fixed.
Even with Italian keyboard everything is fine, reason for which i never realized it was not working.

@marcospassos
Copy link
Author

@asturur Try to switch the keyboard layout to US International - PC. Then, type ~ + a.

@marcospassos
Copy link
Author

If you have any idea of how to fix it, I would be glad to provide a PR to fix it.

@asturur
Copy link
Member

asturur commented Mar 30, 2015

@marcospassos
I uploaded a experimental version here:
http://www.deltalink.it/andreab/fabric/kitchensink.html

Needs fix for copy and pasting.
Before i seriously try to fix it, can you see if it behaves good with accents?

@asturur
Copy link
Member

asturur commented Mar 30, 2015

Also @taveras and @Riddlerrr please if still interested check for chinese input.
Do not check copy and paste ability, just check inputing text, moving around.
Thanks.

@angelim
Copy link

angelim commented Mar 30, 2015

@asturur
Thanks for the effort. I couldn't get accents to work on my mac with US International keyboard using your most recent version of kitchensink.

@asturur
Copy link
Member

asturur commented Mar 30, 2015

i have italian and us keyboard, both works fine.
is a different approach, no event key sniffing at all.

could you please open the console and check what happens when you digit normal letter and accent based one?

@angelim
Copy link

angelim commented Mar 30, 2015

This is the code I used on the "Execute" tab:
text = new fabric.Text('éáíóúãõâêîôûçàèìòù', { left: 100, top: 100 }); canvas.add(text);
It worked as expected: http://cl.ly/image/0h0d183O2J07

When I try to input text directly inside the text element, every accent show up as the letter "a".

@marcospassos
Copy link
Author

@asturur Thanks for the effort. Unfortunately, it's not working yet.

@angelim
Copy link

angelim commented Mar 30, 2015

@asturur, sorry for the incomplete answer above.
I got that result using Chrome Version 41.0.2272.104 (64-bit).
On Firefox 36.0.4 the accents show up, but isolated from the letter they're supposed to be applied to. So the result for "ã" is ~a.
On Safari 7.1.2 it works as expected! :)
As soon as I finish working today I'll take a look at the code and try to diagnose a little further. I have no previous experience with fabric, but I'll give it a shot.

@asturur
Copy link
Member

asturur commented Mar 30, 2015

the code now is messy, so do no waste time, is a proof of concept.
it won t input more than one character. the fact that two letters show up is very strange.

@asturur
Copy link
Member

asturur commented Mar 30, 2015

@marcospassos i installed ubuntu just to test those things.
I do not have apple.
I noticed that in linux ubuntu you have to use a compose KEY to make it work.
On which system are you working? what is your compose key?

Could you give more information about your test case?
Because as of now i can make it work on both windows and linux chrome and firefox.
It still needs a lot of work, because internal routine are changed and style and copy past must be redone.

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

Use this simpler test case, where there is just and itext.

@asturur
Copy link
Member

asturur commented Mar 30, 2015

Also i add, current kitchensink on fabricjs.com does not work with stock android browser (lollipop), while this new logic takes input.

@asturur
Copy link
Member

asturur commented Mar 30, 2015

/cc @vamo89 (original idea of using onInput as far as i remember) @alisspers @MikyKuroneko

could some of you take a test on mac os setup?

@taveras
Copy link

taveras commented Mar 31, 2015

Tested this last page (http://www.deltalink.it/andreab/fabric/test3.html) on a Mac, running 10.10.2 Yosemite, using both the US International keyboard, Chinese - Traditional Keyboard, and Japanese - Hiragana keyboard. I tested this in the latest versions of Chrome, Firefox, and Safari.

The related issue #1529 I would consider resolved with this change. Good work!

@angelim
Copy link

angelim commented Mar 31, 2015

I can also confirm that it's working on a Mac running Mavericks on Chrome, Firefox and Safari. Copy/paste is also working on test3.html. I used US International and Brazil ABNT2.
Thanks for fixing it. Is this going to master or will it be available in another branch?

@marcospassos
Copy link
Author

@asturur now it's working! I'm using Chrome on OSX Yosemite 10.10.2 (US International keyboard).

@asturur
Copy link
Member

asturur commented Mar 31, 2015

good. so after waiting @kangax thoughs i try to solve style related issue.

@vamo89
Copy link

vamo89 commented Apr 2, 2015

Both working for me too. Using Ubuntu - Chrome and Ubuntu - Firefox with Brazil ABNT2 keyboard. Copy and paste working as well. Thanks @asturur.

@kangax kangax added the bug label Apr 6, 2015
@kangax kangax added this to the 1.5.1 milestone Apr 14, 2015
@rianmcguire
Copy link

@asturur Are you able to publish the work you've done so far for your test pages? I would like to finish it up and pull request it.

@jcppman
Copy link

jcppman commented May 27, 2015

This demo http://www.deltalink.it/andreab/fabric/test3.html doesn't work here, my env:
mac osx 10.10.3

tried:

  • firefox 36
  • chrome 43
  • safari 8

with

  • Traditional Chinese
  • Simplified Chinese
  • Japanese

@asturur
Copy link
Member

asturur commented May 27, 2015

i overwrited the demo accidentally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants