Leo Reporter
	Description • 11 years ago

Title: Change in whole base code & algorithm for text prediction and correction in Master Gaia and V1 train 

Currently I m working on leo Gaia i.e V1 train but the MASTER Gaia base code is whole changed and implemented with new algorithm and new feature.
Because of two whole base code and algorithm, its becoming difficult for communicating with Mozilla Engineers to decide where the issues should be solved.

Some issues has been already solved in Master Gaia but cannot be implemented in V1 train because of base code change.This issues need to uplifted in V1 train.
Please check and give your comments.

	Gregor Wagner [:gwagner]
	Comment 1 • 11 years ago

We should move quickly if we want to uplift the new implementation.

	David Flanagan [:djf] Assignee
	Comment 2 • 11 years ago

(In reply to Leo from comment #0)
> Title: Change in whole base code & algorithm for text prediction and
> correction in Master Gaia and V1 train 
> 
> Currently I m working on leo Gaia i.e V1 train but the MASTER Gaia base code
> is whole changed and implemented with new algorithm and new feature.
> Because of two whole base code and algorithm, its becoming difficult for
> communicating with Mozilla Engineers to decide where the issues should be
> solved.

What engineers are you trying to communicate with, and what issues are you trying to resolve.  I'm the engineer who is most familiar with the word suggestion and auto correction code, so you should really bring all such issues to my attention. The only one I know about for the v1.1 release is https://bugzilla.mozilla.org/show_bug.cgi?id=851565, and I'll be fixing that soon.

> 
> Some issues has been already solved in Master Gaia but cannot be implemented
> in V1 train because of base code change.This issues need to uplifted in V1
> train.

What issues are you talking about? Without specific bug numbers, there isn't much to comment on.

> Please check and give your comments.

I also would like to uplift all of the word prediction and auto-correction features from master to v1-train, but as I've commented elsewhere, that is not my decision to make.  The decision to uplift a new feature like auto-correction will be made by the product managers and release managers.  Cc'ing Alex and Chris

	Leo Reporter
	Comment 3 • 11 years ago

(In reply to David Flanagan [:djf] from comment #2)

This issue has been raised in context of the problem due to two base code
like
https://bugzilla.mozilla.org/show_bug.cgi?id=851565
If issues comes on text prediction,there will be two different base codes
where these issues have to be fixed.
For that concern,it was raised if to check Master code can be uplifted to V1.
Thank you.

	Gregor Wagner [:gwagner]
	Comment 4 • 11 years ago

Waiting with the decision also increases risk. The new implementation solves multiple bugs of the old implementation and we are very confident that the new implementation is good to go.
Fixing the old implementation is wasted time imho.

	Tony Chung [:tchung]
	Comment 5 • 11 years ago

Gregor, can you outline the testing risks here?   I'm most concerned about a high number of regressions and integration breakage across apps that touch keyboard work (which is like everything).

	Alex Keybl [:akeybl]
	Comment 6 • 11 years ago

Leo - we're still evaluating this new feature request so we wouldn't like to block on this yet. We're currently testing, and would like to provide a more complete list of remaining work before you make a final decision. It's unclear whether we'll have that testing in time to land for June 6th.

	Alex Keybl [:akeybl]
	Comment 7 • 11 years ago

Moving back to leo? since we don't want this to land automatically until comment 6 is addressed, to prevent automatic uplift.

	Gregor Wagner [:gwagner]
	Comment 8 • 11 years ago

(In reply to Tony Chung [:tchung] from comment #5)
> Gregor, can you outline the testing risks here?   I'm most concerned about a
> high number of regressions and integration breakage across apps that touch
> keyboard work (which is like everything).

I don't have a glass-ball that tells me upcoming bugs with the new implementation.
If we would have landed 4 weeks ago when we asked for the approval I am pretty sure we would have a pretty solid auto-correct implementation by now.

I can only say it the n-th time: We understand the new implementation much better and even if there are small bugs I am sure djf can fix them in no time. And if there are bigger bugs, we can back out. Timing is against us and people that delayed the uplift have to take the risk now.

	Leo Reporter
	Comment 9 • 11 years ago

As suggested by Gregor Wagner,
to keep the issue in leo? ,till the bugs are removed
Changing status to leo? again

	David Flanagan [:djf] Assignee
	Comment 10 • 11 years ago

I generated the attached patch for uplift with these commands:

git checkout v1-train
git pull upstream
git pull upstream v1-train
git checkout -b autocorrect-uplift v1-train
git revert -m1 37d7a
git revert 21ff04
git cherry-pick -x -m1 132c5  # Bug 860462 manual fix to keyboard.css required
git cherry-pick -x -m1 d83b55 # Bug 858158
git cherry-pick -x -m1 23ed3  # Bug 797171
git cherry-pick -x -m1 58665  # Bug 861113
git cherry-pick -x -m1 41348  # Bug 865484
git cherry-pick -x -m1 9fa85  # Bug 868819 manual fix to keyboard.css
git cherry-pick -x -m1 bebcb  # Bug 876658
git cherry-pick -x -m1 6ca1a  # Bug 878387, et. al
git cherry-pick -x -m1 417e5  # Bug 873223 manual fix to keyboard.css
git cherry-pick -x -m1 76116  # Bug 878384
git cherry-pick -x -m1 068905 # Bug 878838
git cherry-pick -x -m1 60d03  # Bug 879076
git cherry-pick -x -m1 bc485  # Bug 878843

Tree of the patches required manual merges for keyboard.css, and I had to add one final commit to tweak the resulting CSS to make the word suggestions display correction on v1-train (because the keyboard layout changed on master).

	David Flanagan [:djf] Assignee
	Comment 11 • 11 years ago

I've just updated the PR with the commit from bug 879187

	David Flanagan [:djf] Assignee
	Comment 12 • 11 years ago

Last one was cherry-picked with git cherry-pick -x -m1 159b0dd

	David Flanagan [:djf] Assignee
	Comment 13 • 11 years ago

Added another commit: 

git cherry-pick -x -m1 eed087

This is is the patch for bug 879533

	David Flanagan [:djf] Assignee
	Comment 14 • 11 years ago

Waiting on one final review for bug 878386 and then I'll push the button and uplift all this!

	David Flanagan [:djf] Assignee
	Comment 15 • 11 years ago

Final commit: added the patch from bug 878386 with

  git cherry-pick -x -m1 0225f1

	David Flanagan [:djf] Assignee
	Comment 16 • 11 years ago

I have approval from akeybl to uplift this to v1-train: https://bugzilla.mozilla.org/show_bug.cgi?id=797170#c34


Merged into v1-train: https://github.com/mozilla-b2g/gaia/commit/13c624690e7bd0a4d4a523f8c0e2c03d574a0a2d

	leo.bugzilla.gecko
	Comment 17 • 11 years ago

In triage