Ubuntu UI Toolkit

Merge lp://staging/~zsombi/ubuntu-ui-toolkit/textinput-text-selection into lp://staging/ubuntu-ui-toolkit/staging

textinput-text-selection
Merge into staging

Proposed by Zsombor Egri on 2014-04-09

Status:

Superseded

Proposed branch:

lp://staging/~zsombi/ubuntu-ui-toolkit/textinput-text-selection

Merge into:

lp://staging/ubuntu-ui-toolkit/staging

Prerequisite:

lp://staging/~zsombi/ubuntu-ui-toolkit/ubuntutestcase-extras

Diff against target:

3993 lines (+2093/-704)

25 files modified

components.api (+8/-2)
modules/Ubuntu/Components/InputHandler.qml (+315/-0)
modules/Ubuntu/Components/Pickers/Dialer.qml (+1/-1)
modules/Ubuntu/Components/Pickers/DialerHand.qml (+1/-1)
modules/Ubuntu/Components/Pickers/DialerHandGroup.qml (+1/-1)
modules/Ubuntu/Components/Pickers/PickerDelegate.qml (+1/-1)
modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml (+1/-1)
modules/Ubuntu/Components/TextArea.qml (+49/-129)
modules/Ubuntu/Components/TextField.qml (+80/-133)
modules/Ubuntu/Components/Themes/Ambiance/TextAreaStyle.qml (+5/-5)
modules/Ubuntu/Components/plugin/plugin.cpp (+3/-0)
modules/Ubuntu/Components/plugin/ucmouse.h (+52/-11)
modules/Ubuntu/Components/plugin/ucmousefilters.cpp (+220/-84)
modules/Ubuntu/Components/qmldir (+1/-0)
modules/Ubuntu/Test/UbuntuTestCase.qml (+2/-0)
tests/resources/inputs/TextInputs.qml (+85/-0)
tests/unit_x11/tst_components/tst_textarea.qml (+320/-60)
tests/unit_x11/tst_components/tst_textarea_in_flickable.qml (+61/-37)
tests/unit_x11/tst_components/tst_textfield.qml (+314/-63)
tests/unit_x11/tst_mousefilters/ForwardComposedEvents.qml (+55/-0)
tests/unit_x11/tst_mousefilters/ForwardEventChained.qml (+51/-0)
tests/unit_x11/tst_mousefilters/HoverEvent.qml (+38/-0)
tests/unit_x11/tst_mousefilters/HoverEvent.qml.moved (+38/-0)
tests/unit_x11/tst_mousefilters/tst_mousefilters.pro (+4/-1)
tests/unit_x11/tst_mousefilters/tst_mousefilterstest.cpp (+387/-174)

To merge this branch:

bzr merge lp://staging/~zsombi/ubuntu-ui-toolkit/textinput-text-selection

Related bugs:

Bug #1168914: TextArea should allow mouse selections if mouse present	Critical	Fix Released
Bug #1183255: [text inputs] Revisit text selection in TextField and TextArea	Critical	Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration	2014-04-09	Needs Fixing on 2014-04-15
Cris Dywan		2014-04-09	Approve on 2014-04-10
Review via email: mp+214928@code.staging.launchpad.net

This proposal supersedes a proposal from 2014-04-03.

This proposal has been superseded by a proposal from 2014-04-15.

Commit message

Fixes TextField and TextArea selection and scrolling behaviors.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-07: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1990/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-07: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1993/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-07: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1995/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-08: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/2000/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-08: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/2002/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-08: Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1001
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/2009/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-trusty-touch/94/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4615/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/957/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/957/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4208/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4739/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/2009/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-09: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/2014/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Cris Dywan (kalikiana) wrote on 2014-04-09: Posted in a previous version of this proposal

I see the same test failure here as J locally.
> tests/unit_x11/tst_components/tst_textarea_in_flickable.qml(72)
> tryCompare(moveSpy, "count", 1, 200);
Why not moveSpy.wait(); here? That seems to fix it for me.

When using select and context menu a few times it's possible to change the selection after the menu opens but before letting go of the mouse button. So I don't get what I selected or worst case nothing.

What I manually checked is that I can select, including scrolling, get a menu, copy, paste and so on, double click to select a word, all of these seem sold aside from the above issue.

FTR toggleFlickablesInteractive looks rather hackish… I guess Repeater doesn't support it, and PropertyChanges doesn't take a list either… but let's have it as-is for now.

Should maybe selectionTimeout.interval be added to modules/Ubuntu/Components/Themes/Ambiance/TextCursorStyle.qml:31

test_press_and_hold_over_selected_text etc. doesn't test the context menu. Especially given the above problem I found while using it, I think we need to test that it can be clicked and catches the right text. Possibly AP.

review: Needs Fixing

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-04-09: Posted in a previous version of this proposal

> I see the same test failure here as J locally.
> > tests/unit_x11/tst_components/tst_textarea_in_flickable.qml(72)
> > tryCompare(moveSpy, "count", 1, 200);
> Why not moveSpy.wait(); here? That seems to fix it for me.

Ahh, right :) I'll fix it.

>
> When using select and context menu a few times it's possible to change the
> selection after the menu opens but before letting go of the mouse button. So I
> don't get what I selected or worst case nothing.

Let's do this in a separate branch..

>
> What I manually checked is that I can select, including scrolling, get a menu,
> copy, paste and so on, double click to select a word, all of these seem sold
> aside from the above issue.
>
> FTR toggleFlickablesInteractive looks rather hackish… I guess Repeater doesn't
> support it, and PropertyChanges doesn't take a list either… but let's have it
> as-is for now.

Well, limitation of PropertyChange, maybe we propose some Change component upstream to deal with a list of targets :).

>
> Should maybe selectionTimeout.interval be added to
> modules/Ubuntu/Components/Themes/Ambiance/TextCursorStyle.qml:31

I will put this in the TextFieldStyle/TextAreaStyle components rather than the cursor style.

>
> test_press_and_hold_over_selected_text etc. doesn't test the context menu.
> Especially given the above problem I found while using it, I think we need to
> test that it can be clicked and catches the right text. Possibly AP.

Yep, but let's get this in a separate branch as well.

Revision history for this message

Cris Dywan (kalikiana) wrote on 2014-04-09: Posted in a previous version of this proposal

Filed bug 1304952 for testing the context menu.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-09: Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1008
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/2016/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-trusty-touch/129
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4675
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/964
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/964
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/964/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/125
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4259
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4259/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5820
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4030
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4802
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4802/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/2016/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/3/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/5/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Cris Dywan (kalikiana) wrote on 2014-04-10:

The menu now stays in place the way it should. Nice!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/10/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-10:

review: Needs Fixing (continuous-integration)

lp://staging/~zsombi/ubuntu-ui-toolkit/textinput-text-selection updated on 2014-04-15

1016. By Zsombor Egri on 2014-04-14: intermediate steps
1017. By Zsombor Egri on 2014-04-14: prerequisite merge
1018. By Zsombor Egri on 2014-04-15: TextArea test fixes
1019. By Zsombor Egri on 2014-04-15: prerequisite merge

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/23/rebuild

review: Needs Fixing (continuous-integration)

lp://staging/~zsombi/ubuntu-ui-toolkit/textinput-text-selection updated on 2014-04-25

1020. By Zsombor Egri on 2014-04-15: prereq merge
1021. By Zsombor Egri on 2014-04-15: cleanup
1022. By Zsombor Egri on 2014-04-15: more cleanup
1023. By Zsombor Egri on 2014-04-16: staging merge
1024. By Zsombor Egri on 2014-04-17: staging merge
1025. By Zsombor Egri on 2014-04-20: replace flick() with mouseDrag()
1026. By Zsombor Egri on 2014-04-20: change flick() with mouseDrag()
1027. By Zsombor Egri on 2014-04-23: versioning merge
1028. By Zsombor Egri on 2014-04-24: tests reworked for tst_textarea_in_flickable.qml
1029. By Zsombor Egri on 2014-04-24: staging merge
1030. By Zsombor Egri on 2014-04-25: staging merge