Closed
Bug 622259
Opened 13 years ago
Closed 13 years ago
In Yahoo Mail! Unable to activate Show Images in Spam Folder
Categories
(Core :: XPConnect, defect)
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: sunveer, Assigned: smaug)
References
()
Details
(Keywords: regression, Whiteboard: [hardblocker] [fx4-fixed-bugday])
Attachments
(1 file)
786 bytes,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:2.0b8) Gecko/20100101 Firefox/4.0b8 Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b8) Gecko/20100101 Firefox/4.0b8 In the screenshot that I have provided in spam there's always a option to show images by clicing on Show Images button in yellow bar but there is no effect on clicking on this button. Reproducible: Always Steps to Reproduce: 1.Open Yahoo mail Spam folder 2. Click on Show Images 3. Actual Results: No effect on clicking.
Build worked : Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101012 Firefox/4.0b8pre http://hg.mozilla.org/mozilla-central/rev/2593c8c8af8b Build broken: Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101013 Firefox/4.0b8pre http://hg.mozilla.org/mozilla-central/rev/29c228a4d7eb Pushlog : http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2593c8c8af8b &tochange=29c228a4d7eb
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•13 years ago
|
||
In local build 1eda0cc3bbf9 : fails 659215248912 : works
Blocks: 580128
Keywords: regression
Updated•13 years ago
|
Component: General → XPConnect
Product: Firefox → Core
QA Contact: general → xpconnect
Target Milestone: --- → mozilla2.0
Version: unspecified → Trunk
Comment 3•13 years ago
|
||
Blake, can you take a look?
blocking2.0: --- → ?
Target Milestone: mozilla2.0 → ---
Comment 5•13 years ago
|
||
Mounir, could you help trace down what's failing here? From the regression range this broke during the massive compartment landing.
Assignee: nobody → mounir.lamouri
blocking2.0: ? → final+
Whiteboard: [softblocker]
Comment 6•13 years ago
|
||
We got this when clicking on the "show images" button: JavaScript error: http://mail.yimg.com/d/combo?/gx/t12a/js/yui_loader/ba87c64b1f20e3e7898b0dc28b2173a6_1.js&/gx/t12a/js/combo/init/fr/75bb21dbefc0df72c250ce29435c9757_1.js&/gx/t12a/js/combo/init/us/ycw.v5b.js&/pim/r/dclient/cg555/js/fr/strings.js&/pim/r/dclient/cg555/js/main/main.js&/bc/bc_2.0.4.js&/pim/r/dclient/cg555/js/openmail_init/openmail_init.js, line 41: Permission denied to access object
Comment 7•13 years ago
|
||
Last good nightly: 2010-10-13 First bad nightly: 2010-10-14 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=178f26e21cfc&tochange=ad0a0be8be74 With no surprise, it's a Compartment JS regression.
Still in Firefox 4 Beta 9 Clicking on Images in Spam folder doesn't have any action.
Comment 11•13 years ago
|
||
Hmm, I didn't see last time that there is a "Not same origin!" warning just before the javascript error. I'm going to check what trigger this warning (assuming the error comes from the warning).
Updated•13 years ago
|
OS: Windows XP → All
Comment 13•13 years ago
|
||
Still in Firefox 4 Beta 4, only for me it is with every mail from yahoo mail, not only with those in the spam folder.
Comment 14•13 years ago
|
||
(In reply to comment #13) > Still in Firefox 4 Beta 4, only for me it is with every mail from yahoo mail, > not only with those in the spam folder. Firefox 4 Beta 10, sorry!
Reporter | ||
Comment 15•13 years ago
|
||
Still in 4 beta 10 bug is there.
Comment 16•13 years ago
|
||
I'm trying to isolate the issue but it's a real pain given that the js code is minified. The only think I've been able to see is with the stack: the "persmission denied" error comes from the click handler AFAICT. Is there anything I could try to make the investigations easier?
Comment 17•13 years ago
|
||
Andreas says he'll look into this. Reassigning.
Assignee: mounir.lamouri → gal
Comment 18•13 years ago
|
||
Do I need the classic yahoo mail or the new one? And I can't find a show images button. Some more detailed STRs would be great.
Comment 19•13 years ago
|
||
Actually never mind I got it: #0 xpc::AccessCheck::deny (cx=0x7fffdc697800, id=...) at ../../../../../js/src/xpconnect/wrappers/AccessCheck.cpp:411 #1 0x00007ffff5b0b042 in CheckAndReport<xpc::OnlyIfSubjectIsSystem> (cx=0x7fffdc697800, wrapper=0x7fffcc7085b0, id=..., act=JSWrapper::GET, perm=@0x7fffffff9cac) at ../../../../../js/src/xpconnect/wrappers/FilteringWrapper.cpp:101 #2 0x00007ffff5b0d6b4 in xpc::FilteringWrapper<JSWrapper, xpc::OnlyIfSubjectIsSystem>::enter (this=0x7ffff7b954a0, cx=0x7fffdc697800, wrapper=0x7fffcc7085b0, id=..., act=JSWrapper::GET) at ../../../../../js/src/xpconnect/wrappers/FilteringWrapper.cpp:149 #3 0x00007ffff6677ac2 in JSWrapper::obj_toString (this=0x7ffff7b954a0, cx=0x7fffdc697800, wrapper=0x7fffcc7085b0) at ../../../js/src/jswrapper.cpp:263 #4 0x00007ffff6619b63 in js::JSProxy::obj_toString (cx=0x7fffdc697800, proxy=0x7fffcc7085b0) at ../../../js/src/jsproxy.cpp:852 #5 0x00007ffff65c3671 in js::obj_toStringHelper (cx=0x7fffdc697800, obj=0x7fffcc7085b0) at ../../../js/src/jsobj.cpp:822 #6 0x00007ffff65c3898 in obj_toString (cx=0x7fffdc697800, argc=0, vp=0x7fffe2efe360) at ../../../js/src/jsobj.cpp:870 #7 0x00007ffff65ad6d0 in js::CallJSNative (cx=0x7fffdc697800, native=0x7ffff65c37a4 <obj_toString>, argc=0, vp=0x7fffe2efe360) at ../../../js/src/jscntxtinlines.h:697 #8 0x00007ffff65a98e4 in js::Invoke (cx=0x7fffdc697800, argsRef=..., flags=0) at ../../../js/src/jsinterp.cpp:700 #9 0x00007ffff6583a13 in js_fun_call (cx=0x7fffdc697800, argc=0, vp=0x7fffe2efe348) at ../../../js/src/jsfun.cpp:2115
Comment 20•13 years ago
|
||
Andreas Gal the beta version of Yahoo mail does not have the problem. Its the regular version of Yahoo mail that replaced Yahoo Classic mail. If you get an email with graphics Yahoo hides the graphics if under the Spam setting for Yahoo mail the "Initially block all images" is enabled.
Comment 21•13 years ago
|
||
So what happens here is that GetRangeParent returns native anonymous content and we wrap that into a system-only wrapper and then throw. Either GetRangeParent should not return this node, or the event code should filter this result. Over to smaug to figure out what to do with this. (gdb) bt 10 #0 GetSelectionClosestFrameForLine (aParent=0x7fffcf1eaa58, aLine=..., aPoint=...) at ../../../layout/generic/nsFrame.cpp:2752 #1 0x00007ffff4e42d3b in GetSelectionClosestFrameForBlock (aFrame=0x7fffcf1eaa58, aPoint=...) at ../../../layout/generic/nsFrame.cpp:2862 #2 0x00007ffff4e42e9e in GetSelectionClosestFrame (aFrame=0x7fffcf1eaa58, aPoint=...) at ../../../layout/generic/nsFrame.cpp:2882 #3 0x00007ffff4e420cc in GetSelectionClosestFrameForChild (aChild=0x7fffcf1eaa58, aPoint=...) at ../../../layout/generic/nsFrame.cpp:2704 #4 0x00007ffff4e43143 in GetSelectionClosestFrame (aFrame=0x7fffe1118020, aPoint=...) at ../../../layout/generic/nsFrame.cpp:2937 #5 0x00007ffff4e435ef in nsIFrame::GetContentOffsetsFromPoint (this=0x7fffe1118020, aPoint=..., aIgnoreSelectionStyle=0) at ../../../layout/generic/nsFrame.cpp:3028 #6 0x00007ffff5216b55 in nsDOMUIEvent::GetRangeParent (this=0x7fffd09440c0, aRangeParent=0x7fffffff9af0) at ../../../../content/events/src/nsDOMUIEvent.cpp:271 #7 0x00007ffff5a90162 in nsIDOMNSUIEvent_GetRangeParent (cx=0x7fffdb247c00, obj=0x7fffd0520688, id=..., vp=0x7fffffffa060) at dom_quickstubs.cpp:15641 #8 0x00007ffff65d7900 in js::CallJSPropertyOp (cx=0x7fffdb247c00, op=0x7ffff5a90019 <nsIDOMNSUIEvent_GetRangeParent>, obj=0x7fffd0520688, id=..., vp=0x7fffffffa060) at ../../../js/src/jscntxtinlines.h:741 #9 0x00007ffff65d80d6 in js::Shape::get (this=0x7fffd41950b8, cx=0x7fffdb247c00, receiver=0x7fffd0520688, obj=0x7fffd0520688, pobj=0x7fffd0c2ce70, vp=0x7fffffffa060) at ../../../js/src/jsscopeinlines.h:256
Updated•13 years ago
|
Assignee: gal → Olli.Pettay
Updated•13 years ago
|
Whiteboard: [softblocker] → [hardblocker]
Assignee | ||
Comment 22•13 years ago
|
||
Um, I wonder why compartments caused this problem.
Assignee | ||
Comment 23•13 years ago
|
||
We have thrown in this case for a long time. Why does it cause a problem now with compartments?
Comment 24•13 years ago
|
||
Andreas thought we leaked native anonymous content. On 3.6 do we definitely throw and not leak? /be
Comment 25•13 years ago
|
||
By "leak" I mean allow access from the page to NAC. /be
Assignee | ||
Comment 26•13 years ago
|
||
On 3.6 we don't give access to NAC. https://bugzilla.mozilla.org/show_bug.cgi?id=475864 "SystemOnlyWrapper"
Assignee | ||
Comment 27•13 years ago
|
||
...but, returning null from nsDOMUIEvent::GetRangeParent does seem to fix Yahoo mail. Did compartments somehow change the behavior of SystemOnlyWrapper?
Assignee | ||
Comment 28•13 years ago
|
||
FF3.6 throws Exception... "Security Manager vetoed action" nsresult: "0x80570027 (NS_ERROR_XPC_SECURITY_MANAGER_VETO)" Trunk Error: Permission denied to access property 'valueOf'
Comment 29•13 years ago
|
||
We should never leak (meaning allow to escape; and wrap and error, but leak is the first bug) NAC, though. Right? /be
Comment 30•13 years ago
|
||
I like my error message better. I can try the old one and see if yahoo eats that, but anyway, the bug fix here is to not hand out NAC, so lets go with that.
Assignee | ||
Comment 31•13 years ago
|
||
Returning null has some regression risk, since then we need to check if some content JS is running, but what if the code calling GetRangeParent is C++, which actually should be able to access NAC.
Assignee | ||
Comment 32•13 years ago
|
||
FYI, making GetRangeParent to throw NS_ERROR_FAILURE doesn't seem to fix Yahoo mail.
Assignee | ||
Comment 33•13 years ago
|
||
I'll post the "return null" patch to tryserver to see if it breaks something there.
Comment 34•13 years ago
|
||
Blake observed that the code is doing Object.prototype.toString.call(NAC) and that used to work. It's an old way to tell the "type" of a JS object, for almost any object: js> NAC=/hi/ /hi/ js> Object.prototype.toString.call(NAC) "[object RegExp]" js> Object.prototype.toString.call(NAC).slice(8,-1) "RegExp" This shows how the web finds all your incompatibilities. /be
Assignee | ||
Comment 35•13 years ago
|
||
http://hg.mozilla.org/try/rev/993d0734e609 seems to pass the tests on tryserver. The problem is that I don't exactly know why the patch helps in this case. The js code is very obscure and my js/xpconnect-debugging-fu could be better. But I can continue tomorrow, unless someone more familiar with jseng/xpconnect could check the js code. Also, it is not clear to me whether we should fix SystemOnlyWrapper handling regression, or just GetRangeParent.
Comment 36•13 years ago
|
||
Is comment 34 accurate? If so, giving a safe toString to SOWs might be worth the trouble. This is one mole we've whacked. How many more? No idea but it's likely too late to find out for some higher value of "for sure", and restore compat for Fx4. If comment 34 in its particulars is not accurate, please say more. /be
Assignee | ||
Comment 37•13 years ago
|
||
I was hoping to see the actual code. But ok, Object.prototype.toString.call(null) does work. I'll post a patch.
Assignee | ||
Comment 38•13 years ago
|
||
I need to write still a testcase for this. The simplest mochitest approach didn't work since that gave too much privileges to event listener, so I guess I need rewrite that as a mochitest which doesn't need to explicitly call netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect") to be able to dispatch events via DOMWindowUtils.
Attachment #507873 -
Flags: review?(jst)
Comment 39•13 years ago
|
||
Comment on attachment 507873 [details] [diff] [review] return null if (parent) { + if (parent->IsInNativeAnonymousSubtree() && + !nsContentUtils::CanAccessNativeAnon()) { + return NS_OK; + } return CallQueryInterface(parent, aRangeParent); } } return NS_OK; Maybe write that as: if (parent && (!parent->IsInNativeAnonymousSubtree || nsContentUtils::CanAccessNativeAnon())) return CallQueryInterface(parent, aRangeParent); } return NS_OK; ? Either way, r=jst
Attachment #507873 -
Flags: review?(jst) → review+
Comment 40•13 years ago
|
||
... with correct bracing, that is.
Comment 41•13 years ago
|
||
Drive-by, jst can take it :-P. The right-heavy if (pooh && (tigger || rabbit || piglet || owl || kanga || roo || eeyore)) { } seems ugly to me. Plus, even though CQI is null-safe I like the way Smaug did it. /be
Comment 42•13 years ago
|
||
I wasn't going for pretty as much as eliminating an avoidable early return. But as I said, I'm fine either way. :)
Comment 43•13 years ago
|
||
CQI is not null-safe, fwiw.
Comment 44•13 years ago
|
||
(In reply to comment #43) > CQI is not null-safe, fwiw. My XPCOM helper memory is shot. Good! /be
Assignee | ||
Comment 45•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/fc363326e0ef
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 47•13 years ago
|
||
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre
Updated•13 years ago
|
Status: RESOLVED → VERIFIED
Whiteboard: [hardblocker] → [hardblocker] [fx4-fixed-bugday]
Comment 48•13 years ago
|
||
Build identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110206 Firefox/4.0b12pre Confirming issue is fixed.
You need to log in
before you can comment on or make changes to this bug.
Description
•