Closed Bug 1662655 Opened 4 years ago Closed 4 years ago

Cannot receive pointer event if enable ui.touch.radius.enabled

Categories

(Core :: Panning and Zooming, defect, P3)

Firefox 82
defect

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox82 --- fixed

People

(Reporter: lonocvb, Assigned: lonocvb, Mentored)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.135 Safari/537.36
Firefox for Android

Steps to reproduce:

  1. enable preference:
    ui.touch.radius.enabled = true
    ui.touch.radius.topmm = 12
    ui.touch.radius.bottommm = 12
    ui.touch.radius.leftmm = 12
    ui.touch.radius.rightmm = 12

  2. html / javascript
    <!DOCTYPE html>
    <html id="html" lang="en">
    <body>

<div id="test"></div>

<style>
#test {
display: inline-block;
background-color: green;
width: 10px;
height: 10px;
}
</style>

<script>
let outer = document.getElementById('html');
outer.addEventListener('click', e => console.log('outer:' + e.type));

let testEle = document.getElementById('test');
testEle.addEventListener('pointerdown', e => console.log('test:' + e.type));  

</script>

</body>
</html>

Actual results:

Click the div, only the 'outer:click' is fired.

Expected results:

click the div, 'test:pointerdown' and 'outer:click' are both fired.

Markdown for the HTML:
html / javascript

<!DOCTYPE html>
<html id="html" lang="en">
<body>

<div id="test"></div>

<style>
#test {
display: inline-block;
background-color: green;
width: 10px;
height: 10px;
}
</style>

<script>
let outer = document.getElementById('html');
outer.addEventListener('click', e => console.log('outer:' + e.type));

let testEle = document.getElementById('test');
testEle.addEventListener('pointerdown', e => console.log('test:' + e.type));  
</script>

</body>
</html>
OS: Unspecified → All
Hardware: Unspecified → All

My patch to fix the bug:

diff --git a/layout/base/PositionedEventTargeting.cpp b/layout/base/PositionedEventTargeting.cpp
index b726e5c..0ccb765 100644
--- a/layout/base/PositionedEventTargeting.cpp
+++ b/layout/base/PositionedEventTargeting.cpp
@@ -151,6 +151,15 @@ static bool HasTouchListener(nsIContent* aContent) {
          elm->HasListenersFor(nsGkAtoms::ontouchend);
 }
 
+static bool HasPointerListener(nsIContent* aContent) {
+  if (EventListenerManager* elm = aContent->GetExistingListenerManager()) {
+    return elm->HasListenersFor(nsGkAtoms::onpointerdown) ||
+           elm->HasListenersFor(nsGkAtoms::onpointerup);
+  }
+
+  return false;
+}
+
 static bool IsDescendant(nsIFrame* aFrame, nsIContent* aAncestor,
                          nsAutoString* aLabelTargetId) {
   for (nsIContent* content = aFrame->GetContent(); content;
@@ -194,7 +203,7 @@ static nsIContent* GetClickableAncestor(
     if (stopAt && content->IsHTMLElement(stopAt)) {
       break;
     }   
-    if (HasTouchListener(content) || HasMouseListener(content)) {
+    if (HasTouchListener(content) || HasMouseListener(content) || HasPointerListener(content)) {
       return content;
     }   
     if (content->IsAnyOfHTMLElements(nsGkAtoms::button, nsGkAtoms::input,
~                                                                                                        

Thanks for the patch! If you could submit it via phabricator (see here) it'd be great.

Kats is familiar with this part of the code, so hopefully he can take a look anyways. The patch looks sensible to me, though it probably also needs a test.

Component: Untriaged → Panning and Zooming
Flags: needinfo?(kats)
Product: Firefox → Core

Indeed, thanks! The patch looks reasonable, although we should also check to make sure the pointer events pref is enabled using StaticPrefs::dom_w3c_pointer_events_enabled() similar to how the touch event function does it. And yes, it would be good to add a test to the existing suite for this feature in this file. Please let me know if you're willing to make these additional changes and submit via phabricator, or if you'd prefer me to do it.

Flags: needinfo?(kats)

I like to do it myself as my first contribution to Mozilla (I am studying about the phabricator and the relavent workflow ... etc). I will add the change about the prefs and test, too. Thanks for your information. :D

Great, don't hesitate to ask if you have any questions or get stuck anywhere!

Mentor: kats
Severity: -- → S3
Priority: -- → P3
Assignee: nobody → lonocvb
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a59319f92fd1
Elements with pointer event are considered clickable. r=kats
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
You need to log in before you can comment on or make changes to this bug.