Closed
Bug 1020847
Opened 11 years ago
Closed 10 years ago
[Keyboard][Handwriting] Support handwriting for Simplified Chinese
Categories
(Firefox OS Graveyard :: Gaia::Keyboard, defect)
Tracking
(b2g-v2.2 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
b2g-v2.2 | --- | fixed |
People
(Reporter: gweng, Assigned: wdeng)
References
Details
Attachments
(3 files, 5 obsolete files)
Here is what Tzu-Wen Lin has done: he ported the open source Tegaki project to JavaScript, and it can read its binary model including about 11,000 Chinese characters in a tolerable waiting time and recognition rate. This also make to train the model in the future to become possible.
The zip for of the app is (too big for Bugzilla):
https://drive.google.com/file/d/0B5cAtIpieNFec19sSnlHMy04NDQ
Now the app is still not an app, and we know there's lots of works to do to become an usable keyboard app. However, people may try it first if you want to, with below caveats:
1. Some characters can't be recognized well, including 火,好,哥 and so on. These characters are also hardly to be recognized on the desktop with original Tegaki IME. This requires more model training to make it better.
2. Please write gently, especially for those small character components like 口 or 日
3. The engine, Zinnia, very depends on the order user write down the character. So if you write a character in wrong order, it would not be recognized successfully.
I'll let Tzu-Wen Lin comments to provide more details :)
Reporter | ||
Comment 1•11 years ago
|
||
Oh, and I've told Tzu-Wen to update the app with two defects, which would be fixed by him later:
1. The app directory now has '_' underscore, which may cause trouble when install it, and downloader can fix it locally
2. There would be a launching cover to tell you the app now is still loading. Now user can just wait about 5 secs to make sure it got loaded completely.
Reporter | ||
Comment 2•11 years ago
|
||
Sorry typo:
Now the app is still not an app
->
Now the app is still not an keyboard app
Comment 3•11 years ago
|
||
IMHO this can be either a independent repo on (1) Github with a web demo or something we land on (2) ./dev_apps/ first. Process in Gaia can be complicated with little benefit if you only looking for releasing the demo as we have stricter landing rules.
Comment 4•11 years ago
|
||
Thanks for your advice. I will change the directory name and add up a loading message.
In order to enhance the usability, the model must be trained and revised. I would like to add this function.
Reporter | ||
Comment 5•11 years ago
|
||
Hi Tzu-Wen: Tim and I have discussed this, and we're thinking of that to put this demo app in an independent repo would be better than land it in Gaia tree, at least before we have resource to maintain and form it as a formal built-in keyboard. Moreover, this project has potential to improve both FirefoxOS and Firefox browser, so to bind with Gaia is not the best way to keep it growing.
So, please create a GitHub repo and put your model separately. Maybe your Handwriting app can be a repo with a Makefile, which would download the model from CDNs like S3, or from the current GoogleDrive. We can also make it better with training functions later.
Flags: needinfo?(wayne2833)
Comment 6•11 years ago
|
||
Actually my idea was having the app demo hosted on Github Pages and download the model from external hosts with CORS XHR. Let's try not to make build script mandatory before we really need it.
PS Once the script downloaded the model for the user, it could save the file in IndexedDB.
Comment 7•11 years ago
|
||
Hi Tzu-Wen, excellent work.
We're implementing a handwriting keyboard for Firefox OS and your work will be much help for it.
Zinnia is the reference recognition engine for the keyboard, as it is light weight, fast and open source (under MIT license). We want to reduce its size to 2MB and limit memory consumption under 4MB, so that it could run on a phone device.
I ported Zinnia C++ engine to Javascript with emscripten (https://github.com/yxl/emscripten-zinnia). The emscripten version has small Chinese model (< 1MB for traditional Chinese), but large engine code (around 800K). And your engine code is quite small, but with a large model file. If you like we can work together to improve it.
Comment 8•11 years ago
|
||
@Greg and @Tim,
There is no open source handwriting engine available for production use. All handwriting IME vendors as I know use proprietary engine. So I think we should make the engine of the handwriting IME replaceable. We will provide an open source engine (such as zinnia) for reference and the device manufacturer could change it later.
The referenced engine could be quite small and light weight, so that we need few efforts to maintain it.
Comment 9•11 years ago
|
||
The demo page is available.( http://waynux.github.io/handwriting/ ) It may take some time to load the model.
@yxl, I am willing to make my contribution. Maybe I have to take a detailed look at the model processing (emscripten-zinnia) first.
Flags: needinfo?(wayne2833)
Reporter | ||
Comment 10•11 years ago
|
||
Hi, I've found that:
1. Model sometime would be stuck when the progress reach about 28MB
2. One error when it completes: Uncaught TypeError: Constructor DataView requires 'new'
I think you may GZip the model file for the web version, if this can make it smaller. However, since the model is a binary file, this may not work.
Flags: needinfo?(wayne2833)
Comment 11•11 years ago
|
||
Thanks for response.
1. Replaced with a light model(50MB->15MB), which is still full Chinese characters supported but less accurate.
2. It should work in Firefox. However, I will fix it soon.
Flags: needinfo?(wayne2833)
Comment 12•11 years ago
|
||
(In reply to Tzu-Wen Lin [:waynux] from comment #9)
> The demo page is available.( http://waynux.github.io/handwriting/ ) It may
> take some time to load the model.
>
> @yxl, I am willing to make my contribution. Maybe I have to take a detailed
> look at the model processing (emscripten-zinnia) first.
Emscripten-zinnia uses the same kind of model files as yours. If possible, I'd like to replace the engine with a derived version from your js engine :-)
BTW, `zinnia_convert`, which is built from zinnia project (zinnia.sourceforge.net), can be used to build and reduce the model file size.
Comment 13•11 years ago
|
||
This is a draft UX proposal made by jingzhang for Firefox OS 1.4.
We made a simple handwriting keyboard based on this design:
https://github.com/yxl/gaia/tree/gaia-1.4
To try the keyboard, flash the modified gaia and add the handwriting keyboard from the built-in keyboard list.
We made the demo keyboard to gather thoughts and feedback about how the keyboard looks like and should work.
Comment 14•11 years ago
|
||
Add up a function "back", which erases the last stroke.
http://waynux.github.io/handwriting/
https://github.com/waynux/zinnia.js
Comment 15•11 years ago
|
||
cc Bruce. If possible, we are expecting this feature can catch up with the train of Firefox OS 2.1.
Reporter | ||
Comment 17•11 years ago
|
||
Well, since there is an UX spec, I think the next step that Tzu-Wen can take is to try to follow the spec to make it as a keyboard app. However maybe we should have a discussion with Bruce again, and to see how to cooperate on this bug (Tim and I have told him about this on Monday).
Comment 18•11 years ago
|
||
(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #17)
> Well, since there is an UX spec, I think the next step that Tzu-Wen can take
> is to try to follow the spec to make it as a keyboard app. However maybe we
> should have a discussion with Bruce again, and to see how to cooperate on
> this bug (Tim and I have told him about this on Monday).
Please see see comment 13 for details.
https://github.com/yxl/gaia/tree/gaia-1.4 is the the implementation based on UX spec. Jing (jingzhang@mozilla.com, UX designer from Beijing office) has provided us the UX Spec, definitely it will be reviewed by Mike's team.
We met Bruce and Mike Tsai last week in Shanghai last week and talked about this implementation and UX review for the next step.
Comment 19•11 years ago
|
||
Attachment #8436640 -
Attachment is obsolete: true
Comment 20•11 years ago
|
||
Hi Bruce, the ux spec is done. Please see how to process this bug. Thanks~
Flags: needinfo?(bhuang)
Comment 21•11 years ago
|
||
(In reply to Jing Zhang from comment #20)
> Hi Bruce, the ux spec is done. Please see how to process this bug. Thanks~
ni Omega to review
(In reply to Yuan Xulei [:yxl] from comment #8)
> @Greg and @Tim,
>
> There is no open source handwriting engine available for production use. All
> handwriting IME vendors as I know use proprietary engine. So I think we
> should make the engine of the handwriting IME replaceable. We will provide
> an open source engine (such as zinnia) for reference and the device
> manufacturer could change it later.
>
> The referenced engine could be quite small and light weight, so that we need
> few efforts to maintain it.
Xulei, just to confirm, are you taking the "replaceable IME engine" approach for this?
Flags: needinfo?(xyuan)
Flags: needinfo?(ofeng)
Flags: needinfo?(bhuang)
Comment 22•11 years ago
|
||
(In reply to Bruce Huang [:bhuang] <bhuang@mozilla.com> from comment #21)
> Xulei, just to confirm, are you taking the "replaceable IME engine" approach
> for this?
Yes, we are.
Flags: needinfo?(xyuan)
Comment 23•11 years ago
|
||
(In reply to Bruce Huang [:bhuang] <bhuang@mozilla.com> from comment #21)
> (In reply to Jing Zhang from comment #20)
> > Hi Bruce, the ux spec is done. Please see how to process this bug. Thanks~
>
> ni Omega to review
Hi Bruce,
I've already reviewed the spec before uploading. It's OK to go.
Flags: needinfo?(ofeng)
Assignee | ||
Comment 24•10 years ago
|
||
Hi Rudy,
The pull request added handwriting IME for Simplified Chinese, including:
1. Added handwriting recognition engine. Now, the recognition engine is Zinnia, all the files are put in apps/keyboard/js/imes/handwriting/hwr/ directory. We can also use other recognition engine by replacing them. There MUST be a file named recognition.js, which encapsulates the handwriting recognition engine and expose Recognition object. The Recognition object must implement setLang and recognize interfaces, handwriting IME engine uses them to communicate with handwriting recognition engine.
2. Added handwriting IME engine(handwriting.js), which can also be used for other languages, including Traditional Chinese. It loads the recognition.js.
3. Added handwriting layout.
4. Added HandwritingPadsManager(handwriting_pads_manager.js), it's responsible for handling events fired on handwriting pads.
5. Modified render.js: render handwriting pad, draw handwritings and clear the handwriting pad.
6. Modified input_method_manager.js: added sendStrokePoints interface, which is handwriting IME used only; Modified activate interface.
7. Added settings for handwriting pad.
Attachment #8469036 -
Flags: review?(rlu)
Assignee | ||
Comment 25•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Hi Omega,
In the setting page of keyboard(handwriting settings part), I made the UI myself, and in symbol pages of handwriting, the switch button (back to handwriting IME default page), maybe I need your visual design for the two places.
Thanks.
Attachment #8469036 -
Flags: ui-review?(ofeng)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → wdeng
Comment 26•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Hi Wei,
Here are some feedback from UX side:
1) When writing something and the finger is still on the pad, now you can move your finger to other keys (e.g. !?,。), or you can press on a key and then start hand writing directly. This is not good UX and please remove this behavior.
2) The stroke looks blurry on Flame (1.5x device). It should be sharp.
3) In Keyboard Settings, Handwriting session should be hidden when there is no handwriting layouts installed.
Carol, please also have some ui-review and help on visual parts which Wei mentioned (back to handwriting and settings), thanks!
Jenny and Arthur, please also give some feedback on Settings if any, thanks!
Attachment #8469036 -
Flags: ui-review?(ofeng)
Attachment #8469036 -
Flags: ui-review?(chuang)
Attachment #8469036 -
Flags: ui-review-
Attachment #8469036 -
Flags: feedback?(jelee)
Attachment #8469036 -
Flags: feedback?(arthur.chen)
Comment 27•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Hello, for handwriting settings, I think we need to provide some sort of feedback to tell user how wide is the stroke, how fast is the response time. Thanks!
Attachment #8469036 -
Flags: feedback?(jelee) → feedback-
Comment 28•10 years ago
|
||
(In reply to Jenny Lee from comment #27)
> Comment on attachment 8469036 [details] [review]
> Patch V1 - pull request 22151
>
> Hello, for handwriting settings, I think we need to provide some sort of
> feedback to tell user how wide is the stroke, how fast is the response time.
> Thanks!
cc Jing to see if she has any idea about this.
Flags: needinfo?(jingzhang)
Comment 29•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Clear the flag per the offline discussion with Omega.
Attachment #8469036 -
Flags: feedback?(arthur.chen)
Comment 30•10 years ago
|
||
(In reply to Yuan Xulei [:yxl] from comment #28)
> (In reply to Jenny Lee from comment #27)
> > Comment on attachment 8469036 [details] [review]
> > Patch V1 - pull request 22151
> >
> > Hello, for handwriting settings, I think we need to provide some sort of
> > feedback to tell user how wide is the stroke, how fast is the response time.
> > Thanks!
>
> cc Jing to see if she has any idea about this.
It's really not easy to show such feedback, especially for the response time... But as my personal experience, I don't care and also have no sense about the exact response time. If I need to modify it, the only thing I want to do, is to set the responsible time faster or lower. So, maybe the feedback is not much necessary.
BTW. I also tried some other handwriting IMEs (Sogou/Baidu/Samsung built-in), but do not find any such feedback either.
Flags: needinfo?(jingzhang)
Comment 31•10 years ago
|
||
Please be informed I may not have the time to review this big patch, as my time would be devoted to wrap up some feature work for v2.1.
Please help ping other reviewers to see if they're available to review this, or you may wait after v2.1 FL (feature landing) so that I could take a look.
Flags: needinfo?(wdeng)
Assignee | ||
Comment 32•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Hi Luke,
Do you have time to take a look?
Thanks.
Attachment #8469036 -
Flags: review?(lchang)
Flags: needinfo?(wdeng)
Comment 33•10 years ago
|
||
Hi Wei,
I can't estimate my ETA but I'll take a look if it's not urgent.
Comment 34•10 years ago
|
||
(In reply to Luke Chang [:lchang] from comment #33)
> Hi Wei,
>
> I can't estimate my ETA but I'll take a look if it's not urgent.
It is not urgent, but is desirable. We have expected it for a long time and
really appreciate your help.
Since it is not a must requirement for v2.1, please take you time.
Assignee | ||
Comment 35•10 years ago
|
||
Hi Omega,
The patch is updated, and UX is improved.
You can try it now, hope for your feedback.
Thanks a lot.
> Here are some feedback from UX side:
> 1) When writing something and the finger is still on the pad, now you can
> move your finger to other keys (e.g. !?,。), or you can press on a key and
> then start hand writing directly. This is not good UX and please remove this
> behavior.
When writing, ignore tapping keys.
> 2) The stroke looks blurry on Flame (1.5x device). It should be sharp.
Now, the stroke looks sharper.
> 3) In Keyboard Settings, Handwriting session should be hidden when there is
> no handwriting layouts installed.
At least one handwriting input method installed and enabled in built-in keyboard, Handwriting session will be shown.
>
> Carol, please also have some ui-review and help on visual parts which Wei
> mentioned (back to handwriting and settings), thanks!
>
> Jenny and Arthur, please also give some feedback on Settings if any, thanks!
Assignee | ||
Updated•10 years ago
|
Attachment #8469036 -
Flags: ui-review- → ui-review?(ofeng)
Comment 36•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Looks that all problems in my previous feedback are fixed.
Attachment #8469036 -
Flags: ui-review?(ofeng) → ui-review+
Comment 37•10 years ago
|
||
However, I found this black panel problem happens sometimes, but I don't quite sure about the STR...
Flags: needinfo?(wdeng)
Assignee | ||
Comment 38•10 years ago
|
||
Yes, I see, the problem maybe caused by gecko, so just filed a new bug 1054929.
(In reply to Omega Feng [:Omega] [:馮於懋] from comment #37)
> Created attachment 8474379 [details]
> blackpanel.png
>
> However, I found this black panel problem happens sometimes, but I don't
> quite sure about the STR...
Flags: needinfo?(wdeng)
Comment 39•10 years ago
|
||
(In reply to Omega Feng [:Omega] [:馮於懋] (PTO until 8/22) from comment #26)
> Comment on attachment 8469036 [details] [review]
> Patch V1 - pull request 22151
>
> Hi Wei,
>
> Here are some feedback from UX side:
> 1) When writing something and the finger is still on the pad, now you can
> move your finger to other keys (e.g. !?,。), or you can press on a key and
> then start hand writing directly. This is not good UX and please remove this
> behavior.
> 2) The stroke looks blurry on Flame (1.5x device). It should be sharp.
> 3) In Keyboard Settings, Handwriting session should be hidden when there is
> no handwriting layouts installed.
>
> Carol, please also have some ui-review and help on visual parts which Wei
> mentioned (back to handwriting and settings), thanks!
>
> Jenny and Arthur, please also give some feedback on Settings if any, thanks!
@Omega,
I cant do UI review right now. I think the overall handwriting IME need a visual refresh.
I'm still working on things that's for 2.1. I would like to know about the schedule for handwriting kb.
@Bruce,
Do you know what's our plan for handwriting keyboard? Do we have schedule for it?
Thanks!
Flags: needinfo?(bhuang)
Comment 40•10 years ago
|
||
There's no hard requirement for a specific release yet. If it cannot be completed in time for 2.1 let's continue working on it for master. If a device ships on 2.1 and needs this we can revisit since picking individual keyboards is relatively low risk.
Flags: needinfo?(bhuang)
Comment 41•10 years ago
|
||
Rudy, do you have any resource to review this? Or should we split it into small patch to review?
Flags: needinfo?(rlu)
Comment 42•10 years ago
|
||
I should have some cycyles to take a look today and next week.
Sorry for the delay.
Flags: needinfo?(rlu)
Comment 43•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
This patch need rebase first.
Please hold off the rebase before I could come up with the better suggestion on how HandwritingPadsManager can interface with reset of the keyboard controlling logic.
Attachment #8469036 -
Flags: review?(rlu)
Attachment #8469036 -
Flags: review?(lchang)
Attachment #8469036 -
Flags: feedback?(timdream)
Comment 44•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
I have leave my feedbacks on Github. Generally my goal is to make sure we have a generalized architecture so that we won't spend more time changing code for future features. I think handwriting logic can all be placed under HandwritingPadManager/HandwritingPadView and the dynamically loaded handwriting IME (and it's worker).
Also, we generally can't r+ any patches without unit tests. Please only set review? if the patch comes with unit tests and with ready-to-land quality. But early feedback? on patches is very welcome -- we want to make sure your effort is spend on the right direction at early on.
Thank you for working on this feature!
Attachment #8469036 -
Flags: feedback?(timdream) → feedback+
Comment 45•10 years ago
|
||
(In reply to wdeng@mozilla.com from comment #35)
> > 3) In Keyboard Settings, Handwriting session should be hidden when there is
> > no handwriting layouts installed.
> At least one handwriting input method installed and enabled in built-in
> keyboard, Handwriting session will be shown.
>
Omega, could you clarify by "install" you mean the section should be show (A) when only when phone/app is shipped with handwriting feature, or (B) when the user enable it?
The patch is currently written like (B) but I think (A) make sense more.
Flags: needinfo?(ofeng)
Comment 46•10 years ago
|
||
Tim, thanks.
Wei is fixing the pull request based on you comments, but hasn't finished yet. He will be on PTO from Sep 29 to Oct 7 and will continue to work on this when back.
Comment 47•10 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (OOO) (MoCo-TPE) (please ni?) from comment #45)
> (In reply to wdeng@mozilla.com from comment #35)
> > > 3) In Keyboard Settings, Handwriting session should be hidden when there is
> > > no handwriting layouts installed.
> > At least one handwriting input method installed and enabled in built-in
> > keyboard, Handwriting session will be shown.
> >
>
> Omega, could you clarify by "install" you mean the section should be show
> (A) when only when phone/app is shipped with handwriting feature, or (B)
> when the user enable it?
>
> The patch is currently written like (B) but I think (A) make sense more.
I meant (A) when only when phone/app is shipped with handwriting feature :)
Flags: needinfo?(ofeng)
Assignee | ||
Comment 48•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
https://github.com/mozilla-b2g/gaia/pull/25213
Assignee | ||
Comment 49•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Hi Tim,
The former pull request has been closed and I created a new pull request. In the new patch, improvements include:
1. Put handwriting recognition engine into a Web Worker.
2. Automatically keep or remove settings for handwriting in building.
3. Adjust code consistently to the latest keyboard framework.
Handwriting pad's position, related to keys, configured in layout, I will file a new bug for it.
Thanks for your review.
Attachment #8469036 -
Attachment description: Patch V1 - pull request 22151 → Patch V2 - pull request 25213
Assignee | ||
Comment 50•10 years ago
|
||
Keep pull request V1 url
Assignee | ||
Updated•10 years ago
|
Attachment #8507622 -
Attachment description: pull request V1 22151 → Patch V1 - pull request 22151
Attachment #8507622 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8469036 -
Flags: feedback+ → feedback?(timdream)
Comment 51•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
Please resubmit the pull request -- the current one was closed and I can't re-open it.
Attachment #8469036 -
Flags: feedback?(timdream)
Assignee | ||
Updated•10 years ago
|
Attachment #8469036 -
Flags: review?(timdream)
Comment 52•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
The attachment still link to pull request 22151. Please re-attach pull request https://github.com/mozilla-b2g/gaia/pull/25213 to Bugzilla.
I've review the patch. The patch misses test scripts, pattern improvements, and need to address some review comments from previous reviews. Please set the feedback flag __on the new attachment__ when you fix these.
Thank you!
Attachment #8469036 -
Flags: review?(timdream)
Assignee | ||
Comment 53•10 years ago
|
||
Comment on attachment 8469036 [details] [review]
Patch V2 - pull request 25213
https://github.com/mozilla-b2g/gaia/pull/25213
Assignee | ||
Comment 54•10 years ago
|
||
Attachment #8469036 -
Attachment is obsolete: true
Attachment #8469036 -
Flags: ui-review?(chuang)
Assignee | ||
Comment 55•10 years ago
|
||
Hi Tim,
The patch is updated, in the new patch, changes include:
1. Add some unit tests.
2. Add some comments.
3. Put handwriting recognition engine in a web worker, followed the pattern in jsZhuyin.
4. Start and stop handwritingPadsManager the same way as other managers.
5. Centralize handing user press move on handwriting pad in a function.
6. Separate handwriting pad creating, rending drawing and clearing in a new file handwriting_pad_view.js.
7. Remove callback and return value directory in drawCanvas function in IMERender.js.
8. Customize handwriting settings in building.
9. Add handwriting settings change listener, update its settings async.
10. Define MAX_RESPONSE_TIME = 1100 instead of using the number directly.
Attachment #8508530 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8511824 -
Flags: feedback?(timdream)
Comment 56•10 years ago
|
||
Comment on attachment 8511824 [details] [review]
Patch V3 - pull request 25213
This is very close to complete except the target object -> view relationship. Well done!
I am not sure all tests are passed, please check it. Also, please think about what we want to do about it when the user rotate the screen. I don't see any handling on that. Thanks.
Attachment #8511824 -
Flags: feedback?(timdream) → feedback+
Assignee | ||
Comment 57•10 years ago
|
||
Comment on attachment 8511824 [details] [review]
Patch V3 - pull request 25213
Hi tim,
The patch has been updated, changes from previous patch includes:
1. change handwriting pad(canvas DOM element) context width the same to device pixel, canvas.width = canvas.style.width * window.devicePixelRatio
2. In IMERender.js,
a) Create a view map, now the relation diagram looks like:
DOM Element <=> target => view
b) Change clearHandwritingPad() to clearHandwritingPad(target)
c) Add setDomElemViewTargetObject function.
d) Adjust handwriting pad UI in resizeUI function, for landscape and portrait modes.
3. Simplify functions in handwriting_pad_view.js and removed buildCanvas and setCanvasHeight functions.
4. Handling multi-handwriting-IME case in handwritingPadsManager.js
Thanks for your review1!
Attachment #8511824 -
Flags: feedback+ → review-
Assignee | ||
Updated•10 years ago
|
Attachment #8511824 -
Flags: review- → review?(timdream)
Comment 58•10 years ago
|
||
Comment on attachment 8511824 [details] [review]
Patch V3 - pull request 25213
The one last outstanding question is here:
https://github.com/mozilla-b2g/gaia/pull/25213/files#r19522269
If this does not result any side effect we are good.
Please also note there is a comment proposing a HandWritingPadTargetHandler.
Attachment #8511824 -
Flags: review?(timdream) → feedback+
Updated•10 years ago
|
feature-b2g: --- → 2.2?
Assignee | ||
Comment 59•10 years ago
|
||
Comment on attachment 8511824 [details] [review]
Patch V3 - pull request 25213
Hi Tim,
The patch is updated, changes include:
1. Removed renderingManager.domObjectMap.set(elem, objRef) in setDomElemViewTargetObject function and the comments above it.
2. Adjusted some lines position in ActiveTargetsManager.prototype._handlePressStart function.
Thanks for your review!
Attachment #8511824 -
Flags: review?(timdream)
Comment 60•10 years ago
|
||
Comment on attachment 8511824 [details] [review]
Patch V3 - pull request 25213
I think this is good enough for us to land it first. Please file three follow-up bugs:
* The "。" symbols beside space has no alternate indicator (the three ...); this is a keyboard render/layout manager bug which affects no only handwriting pad
* The HandwritingPadTargetHandler we talked about -- more research required to better binding the handwriting touch handling w/ current activeTargetManager.
* Removal of _alterKeyboard dead code.
Attachment #8511824 -
Flags: review?(timdream) → review+
Updated•10 years ago
|
Summary: [Keyboard][Handwriting] Support handwriting IME for Traditional, Simplified Chinese and English → [Keyboard][Handwriting] Support handwriting for Simplified Chinese
Comment 61•10 years ago
|
||
wdeng, please file the follow-up bugs?
Bruce, let's take this to 2.2+?
merged in
master: https://github.com/mozilla-b2g/gaia/commit/6f84754adce283ab5002f4105473eab856c0f687
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: needinfo?(wdeng)
Flags: needinfo?(bhuang)
Resolution: --- → FIXED
Comment 62•10 years ago
|
||
sorry had to revert this change since it caused bustages like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=742921&repo=b2g-inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 63•10 years ago
|
||
OK, I will file these bugs.
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #60)
> Comment on attachment 8511824 [details] [review]
> Patch V3 - pull request 25213
>
> I think this is good enough for us to land it first. Please file three
> follow-up bugs:
>
> * The "。" symbols beside space has no alternate indicator (the three ...);
> this is a keyboard render/layout manager bug which affects no only
> handwriting pad
> * The HandwritingPadTargetHandler we talked about -- more research required
> to better binding the handwriting touch handling w/ current
> activeTargetManager.
> * Removal of _alterKeyboard dead code.
Flags: needinfo?(wdeng)
Assignee | ||
Comment 64•10 years ago
|
||
The previous patch was reverted, see bustage:
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=742921&repo=b2g-inbound
This is a new pull request try to solve the problem above, and removed some redundant code as in comment 60
Assignee | ||
Updated•10 years ago
|
Attachment #8511824 -
Attachment is obsolete: true
Comment 65•10 years ago
|
||
Assignee | ||
Comment 66•10 years ago
|
||
Thank you rudy, there are so many test cases there, almost include all platforms.
It seems that the patch passed all the test cases, can it be landed? or any other concern?
(In reply to Rudy Lu [:rudyl] from comment #65)
> Push to try,
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3420701fc4c9
Comment 67•10 years ago
|
||
I think it is good to reland.
master,
https://github.com/mozilla-b2g/gaia/commit/456dabc690a04f58778ad933e32b0c9f22c1bd1d
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
status-b2g-v2.2:
--- → fixed
Resolution: --- → FIXED
Assignee | ||
Comment 68•10 years ago
|
||
@Tim,
Many thanks, I'm a newbie to gaia, and it's also my first time to make such a big patch. I do learn a great from you and appreciate you very much. Thanks for your patience.
@rudy,
You are such a busy bee, thanks a lot for landing the patch.
Comment 69•10 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=07544c780afe
Looks like we have a green here.
Updated•10 years ago
|
feature-b2g: 2.2? → ---
Updated•10 years ago
|
Flags: needinfo?(bhuang)
You need to log in
before you can comment on or make changes to this bug.
Description
•