Last Comment Bug 740252 - EventListenerManager disables various things in RemoveEventListener even if there can be still listeners
: EventListenerManager disables various things in RemoveEventListener even if t...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: 12 Branch
: x86_64 Linux
: -- normal (vote)
: mozilla14
Assigned To: Doug Turner (:dougt)
:
Mentors:
Depends on: 742605
Blocks: 676595 734855
  Show dependency treegraph
 
Reported: 2012-03-28 22:01 PDT by Olli Pettay [:smaug]
Modified: 2012-04-04 18:34 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v.1 (4.55 KB, patch)
2012-03-28 22:45 PDT, Doug Turner (:dougt)
no flags Details | Diff | Review
Patch for bug (4.26 KB, patch)
2012-03-28 22:46 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Review
patch v.2 (4.59 KB, patch)
2012-03-28 23:20 PDT, Doug Turner (:dougt)
no flags Details | Diff | Review
patch v.3 (4.59 KB, patch)
2012-03-28 23:53 PDT, Doug Turner (:dougt)
no flags Details | Diff | Review
patch v.3 (4.59 KB, patch)
2012-03-28 23:57 PDT, Doug Turner (:dougt)
bugs: review+
Details | Diff | Review

Description Olli Pettay [:smaug] 2012-03-28 22:01:53 PDT
One shouldn't call window->Disable* if there are still listeners for those features.
Comment 1 Olli Pettay [:smaug] 2012-03-28 22:03:46 PDT
Oh, and coding style is
if (expr) {
  stmt;
}
Comment 2 Doug Turner (:dougt) 2012-03-28 22:45:28 PDT
Created attachment 610439 [details] [diff] [review]
patch v.1

wip.
Comment 3 Jared Wein [:jaws] (please needinfo? me) 2012-03-28 22:46:00 PDT
Created attachment 610441 [details] [diff] [review]
Patch for bug

Is this enough to fix the bug?
Comment 4 Doug Turner (:dougt) 2012-03-28 23:20:17 PDT
Created attachment 610453 [details] [diff] [review]
patch v.2

style improvements.
Comment 5 Doug Turner (:dougt) 2012-03-28 23:53:24 PDT
Created attachment 610461 [details] [diff] [review]
patch v.3
Comment 6 Doug Turner (:dougt) 2012-03-28 23:57:25 PDT
Created attachment 610464 [details] [diff] [review]
patch v.3
Comment 7 Olli Pettay [:smaug] 2012-03-29 08:22:40 PDT
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] 2012-03-30 13:07:17 PDT
https://hg.mozilla.org/mozilla-central/rev/c85e435f8041

Note You need to log in before you can comment on or make changes to this bug.