Closed Bug 1253989 Opened 8 years ago Closed 8 years ago

Refactor marionette tests for AccessibleCaret

Categories

(Core :: DOM: Selection, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

(Blocks 1 open bug)

Details

Attachments

(6 files, 1 obsolete file)

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.
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/
Attachment #8727321 - Flags: review?(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/
Attachment #8727322 - Flags: review?(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/
Attachment #8727323 - Flags: review?(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/
Attachment #8727324 - Flags: review?(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/
Attachment #8727325 - Flags: review?(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/
Attachment #8727326 - Flags: review?(mtseng)
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 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 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 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 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 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/
Assignee: nobody → tlin
Status: NEW → ASSIGNED
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 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 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 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 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 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/
Attachment #8727326 - Attachment description: MozReview Request: Bug 1253989 Part 6 - Remove touch caret and selection carets naming. r?mtseng → MozReview Request: Bug 1253989 Part 5 - Remove touch caret and selection carets naming. r?mtseng
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/
Attachment #8727486 - Attachment description: MozReview Request: Bug 1253989 Part 7 - Refactor open_test_html(). r?mtseng → MozReview Request: Bug 1253989 Part 6 - Refactor open_test_html(). r?mtseng
Attachment #8727325 - Attachment is obsolete: true
Attachment #8727325 - Flags: review?(mtseng)
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 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 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/
Attachment #8727321 - Flags: review?(mtseng) → review+
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 on attachment 8727324 [details]
MozReview Request: Bug 1253989 Part 4 - Rename AccessibleCaret test files. r?mtseng

https://reviewboard.mozilla.org/r/38439/#review35503
Attachment #8727324 - Flags: review?(mtseng) → review+
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
Attachment #8727326 - Flags: review?(mtseng) → review+
Comment on attachment 8727486 [details]
MozReview Request: Bug 1253989 Part 6 - Refactor open_test_html(). r?mtseng

https://reviewboard.mozilla.org/r/38489/#review35507
Attachment #8727486 - Flags: review?(mtseng) → review+
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
Attachment #8727322 - Flags: review?(mtseng) → review+
Attachment #8727323 - Flags: review?(mtseng) → review+
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
This is a refactor bug, which not affect firefox 47.
You need to log in before you can comment on or make changes to this bug.