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

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

One shouldn't call window->Disable* if there are still listeners for those features.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Oh, and coding style is
if (expr) {
  stmt;
}

Comment 2

[Doug Turner (:dougt)]

Attachment: patch v.1

wip.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug

Is this enough to fix the bug?

Comment 4

[Doug Turner (:dougt)]

Attachment: patch v.2

style improvements.

Comment 5

[Doug Turner (:dougt)]

Attachment: patch v.3

Comment 6

[Doug Turner (:dougt)]

Attachment: patch v.3

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c85e435f8041