Closed Bug 459848 Opened 11 years ago Closed 11 years ago

investigate if the patch for bug 346984 could be backed out

Categories

(Core :: DOM: Events, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

Details

Attachments

(1 file)

I think we could remove the following
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/dom/src/base/nsJSEnvironment.cpp&rev=1.399&mark=1903-1935#1872
Maybe even CheckFunctionAccess before that.

Nowadays nsEventListenerManager::AddScriptEventListener
doesn't have the problem it used to have, because the global object of the 
document is always used.
https://bugzilla.mozilla.org/show_bug.cgi?id=346984&mark=24#c24
The fix was done in bug 349467.
Yeah, I think this can go... Especially if we get some tests in for this stuff.
Attached patch patch + testSplinter Review
The test makes sure that onfoo attributes don't work if there isn't script global object. I verified this also by adding a printf to nsEventListenerManager::AddScriptEventListener - it returned because there wasn't script global.
Not sure what else could be tested here.

The patch backouts bug 346984 - it doesn't do anything else.
Assignee: nobody → Olli.Pettay
Attachment #343115 - Flags: superreview?(bzbarsky)
Attachment #343115 - Flags: review?(bzbarsky)
Could do some manual testing of the original mailnews issue...
Comment on attachment 343115 [details] [diff] [review]
patch + test

Looks good pending that manual test.
Attachment #343115 - Flags: superreview?(bzbarsky)
Attachment #343115 - Flags: superreview+
Attachment #343115 - Flags: review?(bzbarsky)
Attachment #343115 - Flags: review+
Security Error: Content at mailbox:///home/smaug/.thunderbird/jzdttolx.default/Mail/Local%20Folders/test?number=0 may not load data from http://www.guninski.com/mozbugs/mess2.xml#v.
After removing the XBL security check and adding a warning to
event listener manager, I got 
WARNING: No script global object!!! in nsEventListenerManager::AddScriptEventListener

and I also got Security Error: Content at mailbox:///home/smaug/.thunderbird/jzdttolx.default/Mail/Local%20Folders/test?number=0 may not load or link to file:///etc/passwd after hovering over the bound element.

To get comment #5 I had to backout TB's disable-JS-always patch and manually
enable JS.

So even after enabling JS and disabling XBL security check /etc/passwd couldn't be read. I think that is enough testing.
Yeah, that sounds great.  Thank you for testing!
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.