960897 - [Touch Caret] Touch caret sanity test

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[u459114]

Create a marionette test case for touch caret
1. Have an input in page. Default string is "ABC"
2. Sent a mouse event to make caret appear in the end of string. Looks like "ABCI" where I is the position of cursor.
3. Sent drag event to move caret to the begin of this sting, "IABC"
4. Input a character "D"

Verify:
Correct result: Text Content of input element should be "DABC"
Wrong result: "ABCD"

Comment 1

[u459114]

There tests should be involved in this suite
1. Timeout test: start drag touch caret after it disappear 
2. APZC test: make sure view port keep still while dragging touch caret
3. Selection position test: caret should be at correct position after touch caret drag.

Comment 2

[Phoebe Chang [:phoebe]]

Attachment: WIP - touch caret marinette tests

test the timeout, drag to boundary scenario in input element/ textarea/ contenteditable.

Comment 3

[Phoebe Chang [:phoebe]]

Attachment: WIP - touch caret marionette tests

renew WIP

Comment 4

[Phoebe Chang [:phoebe]]

Attachment: WIP - touch caret marionette tests

Comment 5

[u459114]

TY,
Please skip key event input in this bug, since bug 1016184 block this test.
Only verify [sel.rangeCount, sel.anchorOffset, sel.focusOffset] in this sanity test.
File another test case bug, which is blocked by bug 1016184, to complement key-event test.

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

CJ,

I found the solution to Bug 1016147. If it could be fixed in time, we might not need to file another bug.

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: (WIP) Add touch caret marionette tests

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Add marionette test cases for touch caret (v1)

Add marionette test cases for touch caret feature in bug 924692. Test
cases target <input>, <textarea>, and contenteditable elements.

Thanks Phoebe Chang <[e-mail]> for the WIP patch.

Run tests on browser manually:
./mach marionette-test layout/base/tests/marionette/test_touchcaret.py

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=d8f994badc88

Comment 9

[u459114]

Comment on attachment 8432569 [details] [diff] [review]
Add marionette test cases for touch caret (v1)

Review of attachment 8432569 [details] [diff] [review]:
-----------------------------------------------------------------

TingYu, it's great!

::: layout/base/tests/marionette/test_touchcaret.py
@@ +167,5 @@
> +        self.actions.flick(el, src_x, src_y, dest_x, dest_y).perform()
> +
> +        el.send_keys('!')
> +        self.assertEqual('ABCDEFGHI!', self.get_content(el))
> +

before = self.get_content(el) 
el.send_keys('!')
after = self.get_content(el)
self.assertEqual(before + '!', after)

Comment 10

[u459114]

Comment on attachment 8432569 [details] [diff] [review]
Add marionette test cases for touch caret (v1)

Review of attachment 8432569 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/tests/marionette/test_touchcaret.py
@@ +154,5 @@
> +                           touch_caret1_x, touch_caret1_y).perform()
> +
> +        el.send_keys('!')
> +        self.assertEqual('A!BCDEFGHI', self.get_content(el))
> +

The same, try to avoid hard code string here. Make py side not content awareness

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8432569 [details] [diff] [review]
Add marionette test cases for touch caret (v1)

Review of attachment 8432569 [details] [diff] [review]:
-----------------------------------------------------------------

Looks OK to me but I know next to nothing about marionette tests. Please get a review from someone who does.

Comment 12

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Add marionette test cases for touch caret (v2)

CJ,
Great suggestion! I've address your comments by isolating the content from Python side.

Robert,
Thanks for the review. I will find another marionette reviewer.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=07f326f63380

Comment 13

[u459114]

Suggestion:
We may want test case for "preference off" as well.

For example
test_input_move_caret_to_the_right_by_one_character
  target_content = self.get_content(el)
  target_content = content_to_add + target_content << Since touch caret is off, caret stay on the left corner.

Comment 14

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8433046 [details] [diff] [review]
Add marionette test cases for touch caret (v2)

Review of attachment 8433046 [details] [diff] [review]:
-----------------------------------------------------------------

Carry over roc's r+.

CJ, I will see what I can do to add "preference off" test cases.

As for the reason about Mnw failure on try server, I found that I didn't turn on touchcaret preference. I will figure out how to do it.

Comment 15

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8433046 [details] [diff] [review]
Add marionette test cases for touch caret (v2)

Review of attachment 8433046 [details] [diff] [review]:
-----------------------------------------------------------------

looks good so far, will r+ when it passes in try.

::: layout/base/tests/marionette/test_touchcaret.py
@@ +32,5 @@
> +        self._contenteditable = self.marionette.find_element(*self._contenteditable_selector)
> +
> +    def tearDown(self):
> +        # Code to execute after a test are run.
> +        MarionetteTestCase.tearDown(self)

I would move MarionetteTestCase.tearDown(self) to the last line of your tearDown function so your code will be more futureproof if we change its behaviour, ie: If we change MarionetteTestCase's tearDown to delete the marionette session, then the subsequent code will fail.

Comment 16

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

> As for the reason about Mnw failure on try server, I found that I didn't
> turn on touchcaret preference. I will figure out how to do it.

The touch caret preference is indeed on. Mnw is failed because TouchCaret::HandleTouchDownEvent() failed to recognized marionette synthesized touch event id.

Due to the tight schedule, I prefer to enable touch caret tests on browser for now. I've file bug 1020261 to track the enabling of these touch carets test on B2G.

I will upload a refined patch to address reviewer's comments later.

Comment 17

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Add marionette test cases for touch caret (v3, carry roc's r+)

* Add test cases for touch caret disabled.
* Remove tearDown().
* Run tests cases only on browser (Gn).

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=710ca6810779

Comment 18

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

(In reply to Ting-Yu Lin [:TYLin] from comment #17)
> * Run tests cases only on browser (Gn).
Typo. s/Gn/Mn

Comment 19

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8434136 [details] [diff] [review]
Add marionette test cases for touch caret (v3, carry roc's r+)

Review of attachment 8434136 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good but a few tests fail on my machine, which is an os x. I've started a try job to make sure they'll pass:

https://tbpl.mozilla.org/?tree=Try&rev=3f693ecacfef

will r+ when it passes

Comment 20

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Hi Malini,

Thanks for triggering a try job on OS X. They're all passed!

Comment 21

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8434136 [details] [diff] [review]
Add marionette test cases for touch caret (v3, carry roc's r+)

Review of attachment 8434136 [details] [diff] [review]:
-----------------------------------------------------------------

Cool, something must be wrong with my build:) Thanks for doing this!

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/e982377e39a3

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e982377e39a3

Comment 24

[Ed Morley [:emorley]]

Unfortunately I've had to back this out for intermittent test failures:
https://tbpl.mozilla.org/php/getParsedLog.php?id=41196860&tree=Mozilla-Inbound

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/8b2c5a79c186

Comment 25

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Selection carets sanity tests could reuse some utilities from this patch.

Comment 26

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Add marionette test cases for touch caret, r=roc, mdas (v4, carry roc and mdas's r+)

Sorry for causing the intermittent test failures. The failed test case requires the input to scroll a long text to the end. If it scrolls too slow for some reason, the test will fail. I have removed this unreliable test.

Mn is passed several times on different platforms, is should be OK now.
https://tbpl.mozilla.org/?tree=Try&rev=47926fb90a13

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/f6395f17c99f

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f6395f17c99f

Comment 29

[Ed Morley [:emorley]]

Thank you for sorting out the intermittent test failures :-)

Comment 30

[Ed Morley [:emorley]]

Backed out again for bug 1022217 as well as this new one:
https://tbpl.mozilla.org/php/getParsedLog.php?id=41448878&tree=Fx-Team

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/065dcd6cbdc5

Comment 32

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Sorry for introducing the intermittent test failure again. I have some guess of the possible reasons here. https://bugzilla.mozilla.org/show_bug.cgi?id=1022217#c2

I've added some log to reproduce the intermittent test failure, but failed to reproduce any. 
https://tbpl.mozilla.org/?tree=Try&rev=878dfa7402bf

After examining the running time of the failed log in comment 30, I found that each test cases takes about 4~6 seconds to complete which is longer than normal (2~3 seconds). 

Since touch caret will disappear after 3 seconds without any action, I suspect that the test script failed to move touch caret when the try server had heavy loading. I'm going to enlarge the touch caret expiration time so that touch caret won't disappear too quickly during testing.

Comment 33

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Add marionette test cases for touch caret. r=roc, r=mdas (v5, carry roc and mdas's r+)

Enlarge touch caret expiration time to 60 seconds avoid intermittent
test failures in test cases which need to move touch caret.

Try result (Mn):
https://tbpl.mozilla.org/?tree=Try&rev=a615f19ffc4b

Comment 34

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Add marionette test cases for touch caret. r=roc, r=mdas (v6, carry roc and mdas's r+)

Modify commit message only.

Try result (Mn):
https://tbpl.mozilla.org/?tree=Try&rev=a615f19ffc4b

Comment 35

[Ed Morley [:emorley]]

Thank you :-0

Comment 36

[Ed Morley [:emorley]]

:-) even

Comment 37

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I've enlarged the expiration time for touch caret, and have tried Mn on Linux opt and debug at least 20 times in comment 34, they're all green. The orange one does not related to this change. I hope no intermittent test failure happens again.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/c40583e416ee

Comment 39

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c40583e416ee

Comment 40

[Ryan VanderMeulen [:RyanVM]]

Unfortunately, this got uplifted to Aurora prior to comment 30 with the intermittent failures in tow. I pushed an interdiff between that patch and the patch from comment 38 to hopefully stop the failures on Aurora.

https://hg.mozilla.org/releases/mozilla-aurora/rev/750f1205f7a8