1253989 - Refactor marionette tests for AccessibleCaret

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I would like to add new marionette tests for AccessibleCaret in bug 1251519. But before that, I would like to clean up test_accessiblecaret_cursor_mode.py and test_accessiblecaret_selection_mode.py.

Some ideas in my mind:
1. Use @parameterized to generate tests for different elements.
2. Remove all the naming referencing "Touchcaret" and "SelectionCarets" since they were removed.
3. Remove some of the tests which are not used or might be covered by other tests to reduce test running time.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 1253989 Part 1 - Remove tests for accessiblecaret preference off. r?mtseng

I don't feel these tests are helpful. It's unlikely that someone will
accidentally turn on AccessibleCaret on desktop platforms without being
noticed. Remove these tests also reduces the test running time.

Review commit: https://reviewboard.mozilla.org/r/38433/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38433/

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 1253989 Part 2 - Use @parameterized to rewrite cursor mode tests. r?mtseng

Remove _test_touch_caret_hides_after_receiving_wheel_event() completely
since it was a test only for touch caret, which is a leftover in bug
1221459.

Review commit: https://reviewboard.mozilla.org/r/38435/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38435/

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 1253989 Part 3 - Use @parameterized to rewrite selection mode tests. r?mtseng

* Inline some of the open_*_html methods since they're called only once.
* Save test running time by finding the elements needed in tests instead
  of find all of the elements in open_*_html methods.
* Remove test_long_press_to_select_non_selectable_word() since it's
  covered by test_focus_not_changed_by_long_press_on_non_selectable().
* Use hyphen for element ids.
* Replace "left" and "right" caret by "first" and "second" caret,
  respectively.

Review commit: https://reviewboard.mozilla.org/r/38437/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38437/

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 1253989 Part 4 - Rename AccessibleCaret test files. r?mtseng

- Remove "touch caret" and "selection carets" from file names and titles.
- test_selectioncarets_multipleline.html are untouched since I'm going to
  merge it into test_carets_selection.html in next patch.

Review commit: https://reviewboard.mozilla.org/r/38439/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38439/

Comment 5

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 1253989 Part 5 - Make contents become multiple lines in test_selectioncarets.html. r?mtseng

We now make test_minimum_select_one_character() cover multiple line
tests.

Review commit: https://reviewboard.mozilla.org/r/38441/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38441/

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 1253989 Part 5 - Remove touch caret and selection carets naming. r?mtseng

- To avoid confusion, call the blinking cursor (nsCaret) "cursor" so that
  AccessibleCaret can be called caret for short.
- Add second_caret_location() as a helper function, and use it in
  selection mode tests.

Review commit: https://reviewboard.mozilla.org/r/38443/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38443/

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727321 [details]
MozReview Request: Bug 1253989 Part 1 - Remove tests for accessiblecaret preference off. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38433/diff/1-2/

Comment 8

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727322 [details]
MozReview Request: Bug 1253989 Part 2 - Use @parameterized to rewrite cursor mode tests. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38435/diff/1-2/

Comment 9

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727323 [details]
MozReview Request: Bug 1253989 Part 3 - Use @parameterized to rewrite selection mode tests. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38437/diff/1-2/

Comment 10

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727324 [details]
MozReview Request: Bug 1253989 Part 4 - Rename AccessibleCaret test files. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38439/diff/1-2/

Comment 11

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727325 [details]
MozReview Request: Bug 1253989 Part 5 - Make contents become multiple lines in test_selectioncarets.html. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38441/diff/1-2/

Comment 12

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727326 [details]
MozReview Request: Bug 1253989 Part 5 - Remove touch caret and selection carets naming. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38443/diff/1-2/

Comment 13

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 1253989 Part 6 - Refactor open_test_html(). r?mtseng

Review commit: https://reviewboard.mozilla.org/r/38489/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38489/

Comment 14

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727486 [details]
MozReview Request: Bug 1253989 Part 6 - Refactor open_test_html(). r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38489/diff/1-2/

Comment 15

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727321 [details]
MozReview Request: Bug 1253989 Part 1 - Remove tests for accessiblecaret preference off. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38433/diff/2-3/

Comment 16

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727322 [details]
MozReview Request: Bug 1253989 Part 2 - Use @parameterized to rewrite cursor mode tests. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38435/diff/2-3/

Comment 17

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727323 [details]
MozReview Request: Bug 1253989 Part 3 - Use @parameterized to rewrite selection mode tests. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38437/diff/2-3/

Comment 18

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727324 [details]
MozReview Request: Bug 1253989 Part 4 - Rename AccessibleCaret test files. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38439/diff/2-3/

Comment 19

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727326 [details]
MozReview Request: Bug 1253989 Part 5 - Remove touch caret and selection carets naming. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38443/diff/2-3/

Comment 20

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727486 [details]
MozReview Request: Bug 1253989 Part 6 - Refactor open_test_html(). r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38489/diff/2-3/

Comment 21

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727324 [details]
MozReview Request: Bug 1253989 Part 4 - Rename AccessibleCaret test files. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38439/diff/3-4/

Comment 22

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727326 [details]
MozReview Request: Bug 1253989 Part 5 - Remove touch caret and selection carets naming. r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38443/diff/3-4/

Comment 23

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8727486 [details]
MozReview Request: Bug 1253989 Part 6 - Refactor open_test_html(). r?mtseng

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38489/diff/3-4/

Comment 24

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8727321 [details]
MozReview Request: Bug 1253989 Part 1 - Remove tests for accessiblecaret preference off. r?mtseng

https://reviewboard.mozilla.org/r/38433/#review35495

Comment 25

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8727324 [details]
MozReview Request: Bug 1253989 Part 4 - Rename AccessibleCaret test files. r?mtseng

https://reviewboard.mozilla.org/r/38439/#review35503

Comment 26

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8727326 [details]
MozReview Request: Bug 1253989 Part 5 - Remove touch caret and selection carets naming. r?mtseng

https://reviewboard.mozilla.org/r/38443/#review35505

Comment 27

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8727486 [details]
MozReview Request: Bug 1253989 Part 6 - Refactor open_test_html(). r?mtseng

https://reviewboard.mozilla.org/r/38489/#review35507

Comment 28

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8727322 [details]
MozReview Request: Bug 1253989 Part 2 - Use @parameterized to rewrite cursor mode tests. r?mtseng

https://reviewboard.mozilla.org/r/38435/#review35509

Comment 29

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8727323 [details]
MozReview Request: Bug 1253989 Part 3 - Use @parameterized to rewrite selection mode tests. r?mtseng

https://reviewboard.mozilla.org/r/38437/#review35511

Comment 30

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/864342da6536
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc451d25f123
https://hg.mozilla.org/integration/mozilla-inbound/rev/2693625a55e0
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0fdb08d2254
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba096724ac7c
https://hg.mozilla.org/integration/mozilla-inbound/rev/c575320f1d60

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/864342da6536
https://hg.mozilla.org/mozilla-central/rev/dc451d25f123
https://hg.mozilla.org/mozilla-central/rev/2693625a55e0
https://hg.mozilla.org/mozilla-central/rev/c0fdb08d2254
https://hg.mozilla.org/mozilla-central/rev/ba096724ac7c
https://hg.mozilla.org/mozilla-central/rev/c575320f1d60

Comment 32

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

This is a refactor bug, which not affect firefox 47.