Bug 1668870 - Ignore touch listeners in the system group for event retargeting. r=smaug

The audio/video controls element has touch listeners on the scrubber, which
steals events when we don't want it to. So let's ignore those listeners.

Ignoring system group listeners for the purposes of event retargeting seems
reasonable in the general case, because those listeners are coming from the
browser itself. If we're relying on the event retargeting to make those browser
elements easy to hit, then we should not be doing that and instead just make
them bigger.

Test coverage for this change is provided by the android-components tests that
failed in bug 1668112. The next patch re-enables touch event retargeting and
exercises this code in the context of those tests.

Depends on D92436

Differential Revision: https://phabricator.services.mozilla.com/D92437
This commit is contained in:
Kartikaya Gupta 2020-10-06 18:03:25 +00:00
parent 8c7b5f9965
commit 3c8d434ae6
3 changed files with 23 additions and 2 deletions

View File

@ -1462,6 +1462,16 @@ bool EventListenerManager::HasListenersFor(const nsAString& aEventName) const {
}
bool EventListenerManager::HasListenersFor(nsAtom* aEventNameWithOn) const {
return HasListenersForInternal(aEventNameWithOn, false);
}
bool EventListenerManager::HasNonSystemGroupListenersFor(
nsAtom* aEventNameWithOn) const {
return HasListenersForInternal(aEventNameWithOn, true);
}
bool EventListenerManager::HasListenersForInternal(
nsAtom* aEventNameWithOn, bool aIgnoreSystemGroup) const {
#ifdef DEBUG
nsAutoString name;
aEventNameWithOn->ToString(name);
@ -1472,6 +1482,9 @@ bool EventListenerManager::HasListenersFor(nsAtom* aEventNameWithOn) const {
for (uint32_t i = 0; i < count; ++i) {
const Listener* listener = &mListeners.ElementAt(i);
if (listener->mTypeAtom == aEventNameWithOn) {
if (aIgnoreSystemGroup && listener->mFlags.mInSystemGroup) {
continue;
}
return true;
}
}

View File

@ -392,6 +392,11 @@ class EventListenerManager final : public EventListenerManagerBase {
*/
bool HasListenersFor(nsAtom* aEventNameWithOn) const;
/**
* Similar to HasListenersFor, but ignores system group listeners.
*/
bool HasNonSystemGroupListenersFor(nsAtom* aEventNameWithOn) const;
/**
* Returns true if there is at least one event listener.
*/
@ -530,6 +535,9 @@ class EventListenerManager final : public EventListenerManagerBase {
void EnableDevice(EventMessage aEventMessage);
void DisableDevice(EventMessage aEventMessage);
bool HasListenersForInternal(nsAtom* aEventNameWithOn,
bool aIgnoreSystemGroup) const;
public:
/**
* Set the "inline" event listener for aEventName to aHandler. If

View File

@ -161,8 +161,8 @@ static bool HasTouchListener(nsIContent* aContent) {
return false;
}
return elm->HasListenersFor(nsGkAtoms::ontouchstart) ||
elm->HasListenersFor(nsGkAtoms::ontouchend);
return elm->HasNonSystemGroupListenersFor(nsGkAtoms::ontouchstart) ||
elm->HasNonSystemGroupListenersFor(nsGkAtoms::ontouchend);
}
static bool HasPointerListener(nsIContent* aContent) {