Closed
Bug 292464
Opened 19 years ago
Closed 19 years ago
event listeners added using addEventListener() listen only trusted events
Categories
(Core :: DOM: Events, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: smaug, Assigned: smaug)
References
Details
(Keywords: fixed-aviary1.0.5, fixed1.7.9, Whiteboard: need branch landing)
Attachments
(2 files, 2 obsolete files)
6.76 KB,
patch
|
peterv
:
superreview+
chofmann
:
approval1.8b2+
|
Details | Diff | Splinter Review |
11.79 KB,
patch
|
smaug
:
review+
peterv
:
superreview+
jay
:
approval-aviary1.0.5+
shaver
:
approval1.8b3+
|
Details | Diff | Splinter Review |
This is a regression from Bug 289940. Patch coming.
Assignee | ||
Comment 1•19 years ago
|
||
This fixes few things. The size of the nsListenerStruct.mFlags is changed from 8 to 16 bits because NS_PRIV_EVENT_UNTRUSTED_PERMITTED is 0x0800. (I could have changed the event flags macros too, of course) (ls->mFlags & ~NS_PRIV_EVENT_UNTRUSTED_PERMITTED) == aFlags is added so that RemoveEventListener actually does something. AddEventListener methods are changed so that in chrome they add listeners for trusted events, otherwise also other events are handled.
Assignee | ||
Updated•19 years ago
|
Attachment #182281 -
Flags: review?(jst)
Comment 2•19 years ago
|
||
Comment on attachment 182281 [details] [diff] [review] v.1 Good catch, I'm surprised the compilers didn't whine about the mFlags problem. - In nsDocument::AddEventListener(): + nsIURI *docUri; + if ((docUri = NS_STATIC_CAST(nsIDocument*, this)->GetDocumentURI())) { + PRBool isChrome = PR_TRUE; + nsresult rv = docUri->SchemeIs("chrome", &isChrome); Just use mDocumentURI here, no need for a local variable and a call to the inline getter. The rest looks fine to me. r=jst
Attachment #182281 -
Flags: superreview?(peterv)
Attachment #182281 -
Flags: review?(jst)
Attachment #182281 -
Flags: review+
Assignee | ||
Updated•19 years ago
|
Flags: blocking1.8b2?
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•19 years ago
|
Attachment #182281 -
Flags: superreview?(peterv)
Assignee | ||
Comment 3•19 years ago
|
||
Assignee | ||
Updated•19 years ago
|
Attachment #182281 -
Attachment is obsolete: true
Attachment #182341 -
Flags: superreview?(peterv)
Updated•19 years ago
|
Attachment #182341 -
Flags: superreview?(peterv) → superreview+
Assignee | ||
Updated•19 years ago
|
Attachment #182341 -
Flags: approval1.8b2?
Is it possible for chrome documents to have other URI schemes (e.g., jar, file)? Should this instead be a principal check of some sort?
Assignee | ||
Comment 5•19 years ago
|
||
(In reply to comment #4) > Is it possible for chrome documents to have other URI schemes (e.g., jar, file)? > Should this instead be a principal check of some sort? Events are set trusted if the UniversalBrowserWrite is enabled. http://lxr.mozilla.org/seamonkey/source/content/events/src/nsEventStateManager.cpp#4341 But for example XUL script event listeners are checking "chrome" scheme http://lxr.mozilla.org/seamonkey/source/content/xul/content/src/nsXULElement.cpp#665 Which one should be used?
Comment 6•19 years ago
|
||
Comment on attachment 182341 [details] [diff] [review] v1 using mDocumentURI a=chofmann
Attachment #182341 -
Flags: approval1.8b2? → approval1.8b2+
Assignee | ||
Comment 7•19 years ago
|
||
checked in Checking in content/base/src/nsDocument.cpp; /cvsroot/mozilla/content/base/src/nsDocument.cpp,v <-- nsDocument.cpp new revision: 3.549; previous revision: 3.548 done Checking in content/base/src/nsGenericElement.cpp; /cvsroot/mozilla/content/base/src/nsGenericElement.cpp,v <-- nsGenericElement.cpp new revision: 3.385; previous revision: 3.384 done Checking in content/events/src/nsEventListenerManager.cpp; /cvsroot/mozilla/content/events/src/nsEventListenerManager.cpp,v <-- nsEventListenerManager.cpp new revision: 1.200; previous revision: 1.199 done Checking in content/events/src/nsEventListenerManager.h; /cvsroot/mozilla/content/events/src/nsEventListenerManager.h,v <-- nsEventListenerManager.h new revision: 1.75; previous revision: 1.74 done Checking in dom/src/base/nsGlobalWindow.cpp; /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v <-- nsGlobalWindow.cpp new revision: 1.733; previous revision: 1.732 done If a principal check is wanted instead, please reopen.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Flags: blocking1.8b2?
Comment 8•19 years ago
|
||
Yeah, I do think we want that, but that's not critical for 1.8b2. Patch coming.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 9•19 years ago
|
||
Attachment #182447 -
Flags: superreview?(peterv)
Attachment #182447 -
Flags: review?(smaug)
Assignee | ||
Comment 10•19 years ago
|
||
Comment on attachment 182447 [details] [diff] [review] Use the document's principals to check whether it's chrome or not. jst, you added the IsScheme("chrome") check also elsewhere, at least: http://lxr.mozilla.org/seamonkey/source/content/svg/content/src/nsSVGElement.cp p#233 http://lxr.mozilla.org/seamonkey/source/content/base/src/nsGenericElement.cpp#3 375 Those should be fixed too.
Attachment #182447 -
Flags: review?(smaug) → review-
Comment 11•19 years ago
|
||
Yeah, duh. This should get all places in the code where this is needed.
Attachment #182542 -
Flags: superreview?(peterv)
Attachment #182542 -
Flags: review?(smaug)
Updated•19 years ago
|
Attachment #182447 -
Attachment is obsolete: true
Attachment #182447 -
Flags: superreview?(peterv)
Assignee | ||
Updated•19 years ago
|
Attachment #182542 -
Flags: review?(smaug) → review+
Updated•19 years ago
|
Attachment #182542 -
Flags: superreview?(peterv) → superreview+
Blocks: 289940
Updated•19 years ago
|
Attachment #182542 -
Flags: approval1.8b3?
Comment on attachment 182542 [details] [diff] [review] v2: Use the document's principals to check whether it's chrome or not. a=shaver
Attachment #182542 -
Flags: approval1.8b3? → approval1.8b3+
Comment 14•19 years ago
|
||
So is there something left to do here? Or should this be resolved?
Flags: blocking1.8b3?
Assignee | ||
Updated•19 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Flags: blocking1.8b3?
Comment 15•19 years ago
|
||
it seems likely that this caused bug 296764 (arrow keys do not move cursor in input fields with JS disabled).
Updated•19 years ago
|
Flags: blocking1.7.9+
Flags: blocking-aviary1.0.5+
Comment 16•19 years ago
|
||
Comment on attachment 182542 [details] [diff] [review] v2: Use the document's principals to check whether it's chrome or not. Please check this in on the Aviary branch for 1.0.5. a=jay
Attachment #182542 -
Flags: approval-aviary1.0.5+
Updated•19 years ago
|
Whiteboard: need branch landing
Comment 17•19 years ago
|
||
merged into aviary port of patch for bug 289940
Comment 18•19 years ago
|
||
Fix checked into mozilla 1.7 and aviary1.0.1 branches
Keywords: fixed-aviary1.0.5,
fixed1.7.9
You need to log in
before you can comment on or make changes to this bug.
Description
•