Enable "layout.accessiblecaret.enabled_on_touch" in unit tests

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P3
normal
RESOLVED FIXED
8 months ago
2 months ago

People

(Reporter: TYLin, Assigned: TYLin)

Tracking

(Blocks 1 bug)

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

8 months ago
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
Assignee

Comment 1

7 months ago
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
Assignee

Comment 2

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

Depends on D10298
Assignee

Updated

7 months ago
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
Assignee

Comment 3

7 months ago
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
Assignee

Updated

7 months ago
Blocks: 1503715

Comment 4

2 months ago
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

Comment 5

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Assignee

Updated

2 months ago
Blocks: 1539256
You need to log in before you can comment on or make changes to this bug.