Closed Bug 1493317 Opened 6 years ago Closed 6 years ago

Enable "layout.accessiblecaret.enabled_on_touch" in unit tests

Categories

(Core :: DOM: Selection, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

In bug 1485063, "layout.accessiblecaret.enabled_on_touch" was turned off accidentally when moving the preferences. Our tests don't catch the issue because Fennec manually flips the pref on during tests in [1]. Also, we have it turn off in unit testing [2]. We should investigate what happens if [1] and [2] are removed. [1] https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/SelectionActionDelegateTest.kt#203-204 [2] https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/testing/profiles/unittest/user.js#171
The assertion can be reproduced locally by running "./mach test dom/canvas/test/chrome/test_drawWindow_widget_layers.html" with layout.css.accessiblecaret.enabled=true. When AccessibleCaret is enabled, caret elements will be inserted into nsCanvasFrame::mCustomContentContainer, thus it recursively invokes ConstructFramesFromItemList() to construct frames for carets before it had a chance to construct popup group. I feel it's too strict to assume that ConstructFramesFromItemList() cannot be invoke recursively whenever there's a popup group item. Since mHavePendingPopupgroup is set only when the item has mIsRootPopupgroup = true in [1], we should loosen the assertion condition so that it asserts only when the frame construction item has mIsRootPopupgroup set to true. [1] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/layout/base/nsCSSFrameConstructor.cpp#5774-5776
And use correct AccessibleCaret preference to disable it individually in tests. Depends on D10298
Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Summary: Invesigate if it's possible to avoid flip "layout.accessiblecaret.enabled_on_touch" in testing → Enable "layout.accessiblecaret.enabled_on_touch" in unit tests
Hmm, there are some failures geckoview-junit-2, I'm going to disable them, and file a follow-up. https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbc96c27507ab18ff4a65cbb726d1e43385498c6&selectedJob=208729749
Blocks: 1503715
Pushed by aethanyc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/afb8dc106fc6 Part 1 - Fix pending popup group assertion after enabling AccessibleCaret in unittest. r=mats https://hg.mozilla.org/integration/autoland/rev/e19d324e5970 Part 2 - Enable AccessibleCaret in unit tests. r=jchen
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Blocks: 1539256
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: