Closed
Bug 700298
Opened 13 years ago
Closed 13 years ago
BatteryManager should initialize mScriptContext and mOwner
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla10
Tracking | Status | |
---|---|---|
firefox8 | - | unaffected |
firefox9 | - | unaffected |
firefox10 | - | wontfix |
firefox11 | + | fixed |
firefox-esr10 | --- | wontfix |
status1.9.2 | --- | unaffected |
People
(Reporter: smaug, Assigned: mounir)
Details
(Keywords: regression, Whiteboard: [sg:critical][needs branch][qa-])
Attachments
(1 file, 1 obsolete file)
9.23 KB,
patch
|
smaug
:
review+
akeybl
:
approval-mozilla-aurora-
mounir
:
checkin+
|
Details | Diff | Splinter Review |
BatteryManager should initialize mScriptContext and mOwner. Not having the right context for event handlers has traditionally caused security problems. (I'm not sure if that situation has changed, or whether we still need to make sure that right JSContext gets pushed to stack.) Also, BatteryManager should probably extend nsDOMEventTargetWrapperCache, not nsDOMEventTargetHelper. (We're trying get those two merged once there are no nsDOMEventTargetHelper users.)
Assignee | ||
Updated•13 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
Comment 1•13 years ago
|
||
This has led to security problems in the past so let's assume worst case. Either way should fix soon.
Keywords: regression
Whiteboard: [sg:critical]
Comment 2•13 years ago
|
||
Mounir, can you look into this one?
Assignee: nobody → mounir
status-firefox10:
--- → affected
status-firefox11:
--- → affected
status-firefox8:
--- → unaffected
status-firefox9:
--- → unaffected
tracking-firefox10:
--- → +
tracking-firefox11:
--- → +
tracking-firefox8:
--- → -
tracking-firefox9:
--- → -
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #575437 -
Flags: review?(bugs)
Assignee | ||
Updated•13 years ago
|
Whiteboard: [sg:critical] → [sg:critical][needs review]
Reporter | ||
Comment 4•13 years ago
|
||
Comment on attachment 575437 [details] [diff] [review] Patch v1 So if you call navigator.mozBattery twice, you may get an uninitialized BatteryManager. Please assign some value to mBatteryManager only if the initialization succeeded. Also, please make sure *aBattery is set to null if not to a valid BatteryManager.
Attachment #575437 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #575452 -
Flags: review?(bugs)
Assignee | ||
Updated•13 years ago
|
Attachment #575437 -
Attachment is obsolete: true
Reporter | ||
Comment 6•13 years ago
|
||
Comment on attachment 575452 [details] [diff] [review] Patch v1.1 with a comment > class BatteryManager : public nsIDOMBatteryManager >- , public nsDOMEventTargetHelper >+ , public nsDOMEventTargetWrapperCache > , public BatteryObserver As I said in some other bug, nsDOMEventTargetWrapperCache should be the first one to inherit.
Attachment #575452 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•13 years ago
|
Flags: in-testsuite+
Whiteboard: [sg:critical][needs review] → [sg:critical]
Target Milestone: --- → mozilla11
Assignee | ||
Updated•13 years ago
|
Target Milestone: mozilla11 → mozilla10
Assignee | ||
Updated•13 years ago
|
Attachment #575452 -
Flags: checkin+
Assignee | ||
Updated•13 years ago
|
Attachment #575452 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•13 years ago
|
Flags: in-testsuite+ → in-testsuite?
Assignee | ||
Updated•13 years ago
|
Whiteboard: [sg:critical] → [sg:critical][needs branch]
Assignee | ||
Comment 7•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/56bf0f8537fa
Updated•13 years ago
|
status1.9.2:
--- → unaffected
Comment on attachment 575452 [details] [diff] [review] Patch v1.1 with a comment [triage comment] We don't intend to take this feature on Firefox 10 / aurora so we don't need to take this. Denying approval.
Attachment #575452 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Assignee | ||
Comment 9•13 years ago
|
||
This feature *is* in Firefox 10...
Assignee | ||
Updated•13 years ago
|
Attachment #575452 -
Flags: approval-mozilla-aurora- → approval-mozilla-aurora?
Comment 10•13 years ago
|
||
Right, and the intention is to back it out / pref it off I believe.
Assignee | ||
Comment 11•13 years ago
|
||
(In reply to Christian Legnitto [:LegNeato] from comment #10) > Right, and the intention is to back it out / pref it off I believe. It is indeed: there is a patch to pref it off but I believe it's better to have this security bug fixed even if the feature is hidden behind a pref. It doesn't hurt.
Comment 12•13 years ago
|
||
Comment on attachment 575452 [details] [diff] [review] Patch v1.1 with a comment [Triage Comment] Per previous agreement, the battery API is pref'd off in 10 (see bug 703568). This bug is fixed in 11 and will therefore be corrected when the pref is on by default. We do not plan to take any further Battery API changes in 10.
Attachment #575452 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Updated•13 years ago
|
Assignee | ||
Updated•13 years ago
|
Comment 13•13 years ago
|
||
This is fixed everywhere where it needs to be fixed now, right? If so, can we close this bug?
Assignee | ||
Comment 14•13 years ago
|
||
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #13) > This is fixed everywhere where it needs to be fixed now, right? If so, can > we close this bug? This should have been fixed when I pushed the patch to m-c. Sorry about that.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 15•12 years ago
|
||
[Triage comment] This bug is being tracked for landing on ESR branch. Please land patches on http://hg.mozilla.org/releases/mozilla-esr10/by Thursday March 1, 2012 in order to be ready for go-to-build on Friday March 2, 2012. See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more information.
Updated•12 years ago
|
tracking-firefox-esr10:
--- → 11+
Assignee | ||
Comment 16•12 years ago
|
||
(In reply to Lukas Blakk [:lsblakk] from comment #15) > [Triage comment] > > This bug is being tracked for landing on ESR branch. Please land patches on > http://hg.mozilla.org/releases/mozilla-esr10/by Thursday March 1, 2012 in > order to be ready for go-to-build on Friday March 2, 2012. > > See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more > information. I do not understand why we want this to be in ESR given that ESR is using Gecko 10 and Gecko 10 doesn't have Battery API enabled by default. I've been told that I shouldn't push that patch to 10 because of that. Why is that different for ESR?
Comment 17•12 years ago
|
||
[Triage Comment] Sorry, mass modify - closer examination, you're right we don't need this.
tracking-firefox-esr10:
11+ → ---
Updated•12 years ago
|
status-firefox-esr10:
--- → unaffected
Comment 18•12 years ago
|
||
Does this affect Firefox Desktop? Is there anything QA can do to verify this fix?
Whiteboard: [sg:critical][needs branch] → [sg:critical][needs branch][qa?]
Assignee | ||
Comment 19•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #18) > Does this affect Firefox Desktop? Yes. > Is there anything QA can do to verify this fix? That, I don't know. Olli, is there any way to reproduce this?
Reporter | ||
Comment 20•12 years ago
|
||
Some über-tricky JS tests
Comment 21•12 years ago
|
||
qa- unless someone can find a simpler way to verify this fix. Otherwise, we will rely on someone who is already capable of reproducing the issue to verify the fix.
Whiteboard: [sg:critical][needs branch][qa?] → [sg:critical][needs branch][qa-]
Comment 22•12 years ago
|
||
Isn't this more of a "wontfix" for ESR? the feature is there like on 10, we're just relying on people not turning the pref on. For the actual Fx10 we only have to worry about that for 6 weeks, but for the ESR people could flip the pref anytime in the next year if some cool site insists they have a case for it.
Updated•12 years ago
|
Group: core-security
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•