Closed Bug 740252 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: DOM: Events, defect)

12 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: smaug, Assigned: dougt)

References

Details

Attachments

(1 file, 4 obsolete files)

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
Attached patch patch v.1 (obsolete) — Splinter Review
wip.
Attachment #610439 - Flags: review?(bugs)
Attached 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)
Attached 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)
Attached patch patch v.3 (obsolete) — Splinter Review
Attachment #610453 - Attachment is obsolete: true
Attachment #610453 - Flags: review?(bugs)
Attachment #610461 - Flags: review?(bugs)
Attached 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: 12 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.