Closed Bug 960897 Opened 6 years ago Closed 6 years ago

[Touch Caret] Touch caret sanity test

Categories

(Core :: DOM: Selection, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
2.0 S4 (20june)

People

(Reporter: u459114, Assigned: TYLin)

References

Details

Attachments

(1 file, 9 obsolete files)

18.98 KB, patch
TYLin
: review+
Details | Diff | Splinter Review
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"
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.
test the timeout, drag to boundary scenario in input element/ textarea/ contenteditable.
renew WIP
Status: NEW → ASSIGNED
Attachment #8408881 - Attachment is obsolete: true
Attachment #8408897 - Attachment is obsolete: true
Assignee: phchang → tlin
Depends on: 1016147
Blocks: 1016184
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.
CJ,

I found the solution to Bug 1016147. If it could be fixed in time, we might not need to file another bug.
Target Milestone: --- → 2.0 S3 (6june)
Add marionette test cases for touch caret feature in bug 924692. Test
cases target <input>, <textarea>, and contenteditable elements.

Thanks Phoebe Chang <natsuki011077@gmail.com> 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
Attachment #8431389 - Attachment is obsolete: true
Attachment #8432569 - Flags: review?(roc)
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 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 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.
Attachment #8432569 - Flags: review?(roc) → review+
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
Attachment #8432569 - Attachment is obsolete: true
Attachment #8433046 - Flags: review?(mdas)
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 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.
Attachment #8433046 - Flags: review+
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.
Blocks: 1020261
> 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.
* 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
Attachment #8433046 - Attachment is obsolete: true
Attachment #8433046 - Flags: review?(mdas)
Attachment #8434136 - Flags: review+
Attachment #8434136 - Flags: review?(mdas)
(In reply to Ting-Yu Lin [:TYLin] from comment #17)
> * Run tests cases only on browser (Gn).
Typo. s/Gn/Mn
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
Hi Malini,

Thanks for triggering a try job on OS X. They're all passed!
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!
Attachment #8434136 - Flags: review?(mdas) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e982377e39a3
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Selection carets sanity tests could reuse some utilities from this patch.
Blocks: 1019441
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
Attachment #8434136 - Attachment is obsolete: true
Attachment #8436334 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f6395f17c99f
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: 2.0 S3 (6june) → 2.0 S4 (20june)
Thank you for sorting out the intermittent test failures :-)
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Duplicate of this bug: 1022217
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.
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
Attachment #8436334 - Attachment is obsolete: true
Attachment #8438312 - Flags: review+
Attachment #8438312 - Attachment is obsolete: true
Thank you :-0
:-) even
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.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c40583e416ee
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
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
You need to log in before you can comment on or make changes to this bug.