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

RESOLVED FIXED in mozilla14

Status

()

defect
RESOLVED FIXED
8 years ago
8 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)

One shouldn't call window->Disable* if there are still listeners for those features.
Oh, and coding style is
if (expr) {
  stmt;
}
Assignee: nobody → doug.turner
Posted patch patch v.1 (obsolete) — Splinter Review
wip.
Attachment #610439 - Flags: review?(bugs)
Posted patch Patch for bug (obsolete) — Splinter Review
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)
Posted patch patch v.2 (obsolete) — Splinter Review
style improvements.
Attachment #610439 - Attachment is obsolete: true
Attachment #610439 - Flags: review?(bugs)
Attachment #610453 - Flags: review?(bugs)
Posted patch patch v.3 (obsolete) — Splinter Review
Attachment #610453 - Attachment is obsolete: true
Attachment #610453 - Flags: review?(bugs)
Attachment #610461 - Flags: review?(bugs)
Posted patch patch v.3Splinter Review
Attachment #610461 - Attachment is obsolete: true
Attachment #610461 - Flags: review?(bugs)
Attachment #610464 - Flags: review?(bugs)
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
Closed: 8 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.