EventListenerManager disables various things in RemoveEventListener even if there can be still listeners

RESOLVED FIXED in mozilla14

Status

()

Core
DOM: Events
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: smaug, Assigned: dougt)

Tracking

12 Branch
mozilla14
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

5 years ago
One shouldn't call window->Disable* if there are still listeners for those features.
(Reporter)

Comment 1

5 years ago
Oh, and coding style is
if (expr) {
  stmt;
}
(Assignee)

Updated

5 years ago
Assignee: nobody → doug.turner
(Assignee)

Comment 2

5 years ago
Created attachment 610439 [details] [diff] [review]
patch v.1

wip.
Attachment #610439 - Flags: review?(bugs)
Created attachment 610441 [details] [diff] [review]
Patch for bug

Is this enough to fix the bug?
Assignee: doug.turner → jwein
Attachment #610441 - Flags: feedback?(bugs)
Assignee: jwein → doug.turner
Attachment #610441 - Attachment is obsolete: true
Attachment #610441 - Flags: feedback?(bugs)
(Assignee)

Comment 4

5 years ago
Created attachment 610453 [details] [diff] [review]
patch v.2

style improvements.
Attachment #610439 - Attachment is obsolete: true
Attachment #610439 - Flags: review?(bugs)
Attachment #610453 - Flags: review?(bugs)
(Assignee)

Comment 5

5 years ago
Created attachment 610461 [details] [diff] [review]
patch v.3
Attachment #610453 - Attachment is obsolete: true
Attachment #610453 - Flags: review?(bugs)
Attachment #610461 - Flags: review?(bugs)
(Assignee)

Comment 6

5 years ago
Created attachment 610464 [details] [diff] [review]
patch v.3
Attachment #610461 - Attachment is obsolete: true
Attachment #610461 - Flags: review?(bugs)
Attachment #610464 - Flags: review?(bugs)
(Reporter)

Comment 7

5 years ago
Comment on attachment 610464 [details] [diff] [review]
patch v.3


>+nsEventListenerManager::DisableDevice(PRUint32 aType)
>+{
>+  nsPIDOMWindow* window = GetInnerWindowForTarget();
>+  if (!window) {
>+    return;
>+  }
>+  if (aType == NS_DEVICE_ORIENTATION) {
>+    window->DisableDeviceSensor(SENSOR_ORIENTATION);
>+  } else if (aType == NS_DEVICE_MOTION) {
>+    window->DisableDeviceSensor(SENSOR_ACCELERATION);
>+    window->DisableDeviceSensor(SENSOR_LINEAR_ACCELERATION);
>+    window->DisableDeviceSensor(SENSOR_GYROSCOPE);
>+  }
>+}
This could also use switch-case
Attachment #610464 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/c85e435f8041
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Depends on: 742605
You need to log in before you can comment on or make changes to this bug.