1493317 - Enable "layout.accessiblecaret.enabled_on_touch" in unit tests

Status: RESOLVED FIXED

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

Description

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

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

Comment 1

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

Attachment: Bug 1493317 Part 1 - Fix pending popup group assertion after enabling AccessibleCaret in unittest.

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

Comment 2

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

Attachment: Bug 1493317 Part 2 - Enable AccessibleCaret in unit tests.

And use correct AccessibleCaret preference to disable it individually in tests.

Depends on D10298

Comment 3

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

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
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

Comment 5

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/afb8dc106fc6
https://hg.mozilla.org/mozilla-central/rev/e19d324e5970