Closed Bug 462428 Opened 16 years ago Closed 16 years ago

Regression preventing Minefield from working with latest Hotmail upgrade

Categories

(Core :: XPConnect, defect, P1)

Product:
Core
Component:
XPConnect
Platform:
x86
All
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.2a1

People

(Reporter: fehe, Assigned: mrbkap)

References

(Blocks 1 open bug,
URL
)

Details

(Keywords: common-issue-, regression, verified1.9.1)

Attachments

(2 files, 4 obsolete files)

file>save as; complete web page for debugging.
360.00 KB, application/x-tar
Details
Fixing Bill's issue
20.29 KB, patch
jorendorff
: review+
mrbkap
: superreview+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081030 Firefox/2.0.0.11
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081030 Firefox/2.0.0.11

A regression since the August 31 nightly build prevents Firefox from working with the latest Hotmail upgrade.

Regression range:
Pass: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080830031750 Minefield/3.1b1pre

Fail: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080831162801 Minefield/3.1b1pre


Reproducible: Always

Steps to Reproduce:
1. Create a new profile
2. Go to about:config and change the value of general.useragent.extra.firefox to Firefox/3.0.3
3. Go to www.hotmail.com and login
4. If your hotmail account has been upgraded to the new look, you will see a message titled, "Your inbox has a slightly new look."  Click the "Continue" button.
5. If you're not already in your Inbox, click Inbox, on the left.
6. Now you should see a yellow "Loading..." message box, at bottom left, that remains.
7. Try clicking any of the folders (i.e. Inbox, Junk, Drafts, etc.) on the left.  None work.
8. If you have any messages in your Inbox, try clicking any of them.  None is accessible.
9. Try clicking "Mark as", "Move to", or "Option", at the top.  None work.
Keywords: regression
Version: unspecified → Trunk
Summary: Regression preventing Minefield from working with Hotmail upgrade → Regression preventing Minefield from working with Hotmail latest upgrade
Summary: Regression preventing Minefield from working with Hotmail latest upgrade → Regression preventing Minefield from working with latest Hotmail upgrade
I can confirm this.
Flags: blocking-firefox3.1?
also confirmed this
Status: UNCONFIRMED → NEW
Ever confirmed: true
Still occurs with Minefield 3.1b2pre
OS: Windows XP → All
This also happens on the pre-alpha versions of SeaMonkey.
Link with check-ins in that time frame:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-08-30+02%3A00%3A00&enddate=2008-08-31+17%3A00%3A00
Also looks like a Core issue
Flags: blocking-firefox3.1?
Product: Firefox → Core
QA Contact: general → general
(In reply to comment #8)
> Link with check-ins in that time frame:
> http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-08-30+02%3A00%3A00&enddate=2008-08-31+17%3A00%3A00
> Also looks like a Core issue

There's been a lot of reports about new hotmail not working, and this could possibly be the regression range.  Can we have a layout developer take a look please?   

Flagging blocking1.9.1? and relnote as its a highly visible area.
Flags: blocking1.9.1?
Whiteboard: relnote
Can someone please "Save page as" the web page, zip it up, and attach the zip to this bug?
I want to add as well that this is not hotmail specific.  This also affects Mibbit.com AJAX irc client.
blocking1.9.1+=damons
Flags: blocking1.9.1? → blocking1.9.1+
Assignee: nobody → jorendorff
This is due to quick stubs.  The page depends on something.prototype.__lookupSetter__("display") returning a function. We intentionally broke that.
For more about the decision to break this, see the first bullet point in bug 407216 comment 38 and the subsequent total lack of anyone caring.

The code that's broken is like this:

    var c = window.CSSStyleDeclaration;
    c._oldDisplay = c.prototype.__lookupSetter__("display");
    c.prototype.__defineSetter__("display",function(a) {
        c._oldDisplay.call(this,a);
        b();
      });

Are we going to fix this?
(In reply to comment #15)
> For more about the decision to break this, see the first bullet point in bug
> 407216 comment 38 and the subsequent total lack of anyone caring.
> 
> The code that's broken is like this:
> 
>     var c = window.CSSStyleDeclaration;
>     c._oldDisplay = c.prototype.__lookupSetter__("display");
>     c.prototype.__defineSetter__("display",function(a) {
>         c._oldDisplay.call(this,a);
>         b();
>       });
> 
> Are we going to fix this?

Can you assign the bug to the correct component?  It's probably not getting looked at as much here in Core::General.
-> Component: XPConnect.

Unassigning, too. I personally consider it WONTFIX.
Assignee: jorendorff → nobody
Component: General → XPConnect
QA Contact: general → xpconnect
Why would a bug that affects multiple sites that use Ajax be considered WONTFIX?
Maybe I don't understand the term WONTFIX, but it sounds like a ridiculous idea unless there is another way this is being addressed?
If I can't use hotmail, I won't use firefox.  Why would we have a regression and call it WONTFIX.
Hotmail works with Chrome and IE so I think this is a problem with Firefox
I have hit this as well

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1

If WONTFIXing, has evangelism done anything to fix this with hotmail.com?
I might be ignorant enough but I can't use hotmail not my uncle's carpentry website with 10 visits a month

Looking forward to see if the beta2 fixes this but my nightly did not work either
(In reply to comment #25)
> If WONTFIXing, has evangelism done anything to fix this with hotmail.com?

The bug isn't marked WONTFIX (yet) - that was just a comment from one person. I would assume nothing has been done by way of evangelism.  The bug is marked as a blocker, so someone will look at it at some point.

> Looking forward to see if the beta2 fixes this but my nightly did not work
> either

Beta 2 will be the exactly the same code as the nightly from a couple of days ago (unless something comes up that delays the beta), so you don't have much to look forward to.
I guess this problem was solved by Microsoft ... they updated their Hotmail service and it's working here on different machines with the newest nightly (XP SP3 and Vista SP1)
(In reply to comment #27)
> I guess this problem was solved by Microsoft ... they updated their Hotmail
> service and it's working here on different machines with the newest nightly (XP
> SP3 and Vista SP1)

I just tried with the latest nightly and it still having the same problem.  Did you do anything special?
I just tried with 3.1b2 and it doesn't work.  They key is you need to click on messages to delete or navigate folders.
I also tried with the last SeaMonkey nightly (Buil ID:20081126001314) and has the same problem.
Well that's strange - it's not working anymore!?! I'm really sure it worked some hours ago but now there are the same problems as before ... But I guess it's not a bug of Firefox and we should wait what Microsoft fixes in this update: http://windowslivewire.spaces.live.com/Blog/cns!2F7EB29B42641D59!23799.entry
(In reply to comment #31)
> Well that's strange - it's not working anymore!?! I'm really sure it worked
> some hours ago but now there are the same problems as before ... But I guess
> it's not a bug of Firefox and we should wait what Microsoft fixes in this
> update:
> http://windowslivewire.spaces.live.com/Blog/cns!2F7EB29B42641D59!23799.entry

It's a bug in FireFox, it affects more than just hotmail.
I don't understand why people keep saying this is not a bug in Firefox.  It's a regression and has already been acknowledged (see Comment #15) to be caused by  Bug 407216.  Is it intentionally regressed because of a change in standards?  If not, it's a bug.

If Hotmail is broken because Microsoft is doing something non industry supported, then it's a Hotmail bug.  But if Hotmail is broken because Mozilla has decided to stop supporting legitimate functionality, it most definitely is a Firefox bug--whether Microsoft comes up with a workaround or not.

And if the latter is the case, this most likely is not the last time Firefox will fail; then so much for supporting industry standards.
I found a workaround to look at your emails. Just select the message you want to see then click the printer button. It pops up a new window showing the full content of the email, including downloading attachments files and clicking on URL-linked text/images.

If you want to switch folders (Inbox, Junk, Sent,...), click on manage folders then select the folder you want to access.

Tested on Minefield 3.1b3pre/20081126
Please fix this bug, it's still here.  People are not going to be happy when Firefox 3.1 hits final and they can't access their Hotmail.
same here.  Firefox 3.1 beta 2 and Seamonkey 2.0 alpha 1 both hang when attempting to access Hotmail messages from the Hotmail window.  The problem did NOT occur when I was using Firefox 3.0.4 and Seamonkey 1.1.13.  Definitely a problem with any Gecko 1.9.1.x based web browsers.
(In reply to comment #32)
> (In reply to comment #31)
> > Well that's strange - it's not working anymore!?! I'm really sure it worked
> > some hours ago but now there are the same problems as before ... But I guess
> > it's not a bug of Firefox and we should wait what Microsoft fixes in this
> > update:
> > http://windowslivewire.spaces.live.com/Blog/cns!2F7EB29B42641D59!23799.entry
> 
> It's a bug in FireFox, it affects more than just hotmail.

A bug only in Firefox 3.1, Trel, not in other versions of Firefox like 3.0 and earlier which Hotmail worked correctly on.
(In reply to comment #37)
> (In reply to comment #32)
> > (In reply to comment #31)
> > > Well that's strange - it's not working anymore!?! I'm really sure it worked
> > > some hours ago but now there are the same problems as before ... But I guess
> > > it's not a bug of Firefox and we should wait what Microsoft fixes in this
> > > update:
> > > http://windowslivewire.spaces.live.com/Blog/cns!2F7EB29B42641D59!23799.entry
> > 
> > It's a bug in FireFox, it affects more than just hotmail.
> 
> A bug only in Firefox 3.1, Trel, not in other versions of Firefox like 3.0 and
> earlier which Hotmail worked correctly on.

I know, I was saying Hotmail isn't the only site affected.  It's a Firefox bug, not a Hotmail bug.
(In reply to comment #38)
> (In reply to comment #37)
> > (In reply to comment #32)
> > > (In reply to comment #31)
> > > > Well that's strange - it's not working anymore!?! I'm really sure it worked
> > > > some hours ago but now there are the same problems as before ... But I guess
> > > > it's not a bug of Firefox and we should wait what Microsoft fixes in this
> > > > update:
> > > > http://windowslivewire.spaces.live.com/Blog/cns!2F7EB29B42641D59!23799.entry
> > > 
> > > It's a bug in FireFox, it affects more than just hotmail.
> > 
> > A bug only in Firefox 3.1, Trel, not in other versions of Firefox like 3.0 and
> > earlier which Hotmail worked correctly on.
> 
> I know, I was saying Hotmail isn't the only site affected.  It's a Firefox bug,
> not a Hotmail bug.


Would you care to report those sites affected instead of repeating "Its a Firefox Bug"?

That will go along way to prove your point a lot easier than spamming the bug with the same thing over and over.
I already did, on Nov 11th
Assignee: nobody → mrbkap
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1b3
Confirmed still a problem.
Blocks: 407216
Everybody - thanks for the investigation. We know this is a problem and are going to fix it soon. Please stop commenting, though, since it clutters up the bug and sends a bunch of people additional e-mail.
You can make hotmail work 100% properly in minefield by installing IE-TAB from:

https://addons.mozilla.org/en-US/firefox/addon/1419

then go to Tools- IE tab options and add the following sites to the sites filter:

YOU MUST COPY THEM DOWN,EXACTLY AS BELOW.
INCLUDE ALL *s

http://*hotmail.com/*
http://*mail.live.com*

Ta-da.

Yes this is a cheap work-around as you are basically using IE but within minefield.

Also I know I shouldn't really be commenting, but until it gets fixed ppl can use this.
That has no chance of working for Linux or Mac users.
It also doesn't help for Seamonkey.
Keywords: common-issues+
I confirm the workaround works for Firefox 3.1 Beta 2
with 10 bugs filed as duplicate of this, why don't we fix this?
jmaher:  I think comment #42 suggests we are (or will be) attempting to.
(In reply to comment #50)
> with 10 bugs filed as duplicate of this, why don't we fix this?
Please read comment 42.  Comments like yours just take away from developer time because they have to read it, making the fix take even longer.  This bug is marked as blocking-1.9.1, which means it will get fixed before Firefox 3.1 is released.  It's called Minefield for a reason - things break.  If you cannot deal with that, you should not be using nightlies.

Unless someone has something new to add to this bug, they shouldn't be commenting.
Target Milestone: mozilla1.9.1b3 → mozilla1.9.1
Same issue with Mint.com. It looks like a javascript issue, and you are unable to access any links that require javascript. I hope that helps.
(In reply to comment #53)
> Same issue with Mint.com. It looks like a javascript issue, and you are unable
> to access any links that require javascript. I hope that helps.
That's not this, that is bug 469627.
Attached patch patch v.5 (obsolete) — — Splinter Review 
I fiddled around for a while with this patch today trying to limit it correctly. In particular, we don't really want to expose arbitrary JSPropertyOps to scripts. While this shouldn't cause any particular security holes that we know about, it leaks information about the underlying implementation that could be used in evil ways.

Originally, I had an implementation of lookup[GS]etter per-interface, that would then pass its _properties into the actual new version of __lookup[GS]etter__, and that function would vet the found JSPropertyOp against that list. However, multiple inheritance defeated me, leading to the current scheme, where all quickstubbed prototypes that define properties grow a (non-enumerable) __lookup[GS]etter__ magically and then we ensure that the object we find the JSPropertyOp on is actually an XPConnect prototype. Since (to my knowledge) XPConnect does not use JSPropertyOps anywhere else, this is an effective test to protect ourselves while restoring desired compatibility.

Note that one limitation of this approach is that something like: Object.prototype.__lookupGetter__.call(document, 'documentElement') will still return undefined. I think (and others agree) that if you go through that much trouble, you get what you deserve.

I still need to test this thoroughly and write mochitests (thus the version .5 number in the patch description), but this should be good for review now. The mochitests will come tomorrow.
Attachment #353609 - Flags: review?(jorendorff)
Attached patch patch v1 (obsolete) — — Splinter Review 
This includes a mochitest and actually works (as opposed to the previous patch, which only worked far enough to let hotmail not have any errors).

I *really* wanted to make the returned function be a bound method (which I tried, but was broken in attachment 353609 [details] [diff] [review] because it was also a fast native), but hotmail shows that it can't be, since they do |c.prototype.__lookupSetter__(...)| meaning that we'd bind the returned setter to |c.prototype|, which throws when they try to use it.
Attachment #353609 - Attachment is obsolete: true
Attachment #353751 - Flags: review?(jorendorff)
Attachment #353609 - Flags: review?(jorendorff)
Comment on attachment 353751 [details] [diff] [review]
patch v1

Since you can assign to __proto__, I guess you could already apply quick-stubbed getters and setters to objects of other types.

I thought I remembered there being something dangerous about exposing these PropertyOps to script, specifically to do with scripts being able to pass arbitrary 'this' values.  But having looked back at qsgen.py and xpcquickstubs.cpp, I think this is safe.

>+    JSPropertyOp pop = reinterpret_cast<JSPropertyOp>(JSVAL_TO_PRIVATE(v));
...
> +    NS_ASSERTION((jsval(wanted) & 1) == 0,
> +                 "Misaligned function pointer, crashes to follow");

:-P  I'm not fond of this.  I wonder if it is possible to build on x86 in such a way as to trip this assertion.  It seems possible, since -falign-functions is an option...  If so, it makes sense to make this a real runtime check instead of an assertion (throwing an error if the pointer is odd, rather than crashing later).

> +    if(!JS_ValueToId(cx, id, &interned_id) ||
> +       !JS_LookupPropertyWithFlagsById(cx, obj, interned_id,
> +                                       JSRESOLVE_QUALIFIED, &obj2, &v) ||
> +       (obj2 &&
> +        !JS_GetPropertyAttrsGetterAndSetter(cx, obj2, name, &attrs, &found,
> +                                           &getter, &setter)))
> +        return JS_FALSE;

You know better than I do -- does (possibly non-native) obj2 need to be rooted here?

If we want to support C.prototype.__lookupGetter__, it's worth testing that.

r+ with that change (and whatever else you want to take from these comments).
Attachment #353751 - Flags: review?(jorendorff) → review+
How is this bug affected by http://www.computerworld.com/action/article.do?command=viewArticleBasic&articleId=9124092
"December 19, 2008 (Computerworld)  Microsoft Corp. yesterday rolled back a recent change to Windows Live Hotmail, more than a month after users had complained bitterly about a new interface the company unveiled in September."
"We heard many users say that they had trouble navigating through Hotmail, especially if they had a smaller monitor," Microsoft's Hotmail team wrote on its company blog. "We've decided to make a significant change in our product: Hotmail will scroll like classic Hotmail."
"Users can revert to Hotmail's earlier scrolling behavior by clicking the "Inbox" folder, clicking "Options" in the upper right, then under "Reading pane settings," selecting "Off.""
I doubt Microsoft's change will change anything major to Hotmail. Therefore, I think many will still complain, the bug will still be there and we will get our Hotmail bug fixed by the developer here.
The patch has been included in these builds: http://www.wg9s.com/mozilla/firefox/

I've tested the Linux version and it seems to work except of some things:

1. the button "mark as" (nothing happens)
2. the button "move" (nothing happens)
3. the button "options" (nothing happens)
4. the help-button right of "option" (nothing happens)

I didn't watched if there are entries in the error console yet, I'm sorry. I hope these informations are helpful for you. (Have a nice Christmas Eve)
I did some more investigation on this.

First, I created a build where the patch here was the only extra patch applied to rule out any regression the could be caused by any of the other patches I include in my daily builds.  The issue reported in comment #61 existed in that builds as well.

Secondly, I observed that all of the buttons that don't work have drop-down menus associated with them.

I then found that none of the pulldown menus from the Windows Live toolbar (More MSN and the identity choice) work either.  However, this toolbar appears throughout the Windows Live site, but it is only in the mail.live.com area of the site that the menus do not work.
I spun off this issue into bug 471184, which I filed as a tech evangelism bug since everything seems to work fine if you change your user-agent string to identity the browser as Safari.

Oddly, doing that fixes this bug as well, so if the only reason to include this patch is to fix hotmail, then getting Microsoft to just use the Safari code for the Mozilla case would seem to be a better solution.

I suspect, however, that the issue here should be addressed independent of the Hotmail issue.
(In reply to comment #63)
> I spun off this issue into bug 471184, which I filed as a tech evangelism bug
> since everything seems to work fine if you change your user-agent string to
> identity the browser as Safari.
> 
> Oddly, doing that fixes this bug as well, so if the only reason to include this
> patch is to fix hotmail, then getting Microsoft to just use the Safari code for
> the Mozilla case would seem to be a better solution.
> 
> I suspect, however, that the issue here should be addressed independent of the
> Hotmail issue.

I'm not so sure that is accurate.
Using a stable firefox, but spoofing my useragent to be in the non-working range, both hotmail AND the Windows live toolbar work fine.  So whatever code they're sending to Firefox there, did still work in 2.x and 3.0.x
(In reply to comment #64)

> I'm not so sure that is accurate.
> Using a stable firefox, but spoofing my useragent to be in the non-working
> range, both hotmail AND the Windows live toolbar work fine.  So whatever code
> they're sending to Firefox there, did still work in 2.x and 3.0.x

I never said they were sending different code for new versions of Firefox than older versions.
(In reply to comment #65)
> (In reply to comment #64)
> 
> > I'm not so sure that is accurate.
> > Using a stable firefox, but spoofing my useragent to be in the non-working
> > range, both hotmail AND the Windows live toolbar work fine.  So whatever code
> > they're sending to Firefox there, did still work in 2.x and 3.0.x
> 
> I never said they were sending different code for new versions of Firefox than
> older versions.

I never said that either.
You said that a solution is for Microsoft to send the Safari code to Firefox.

I'm saying that the code they send for Firefox WORKS on the older Firefoxes, so while that may be a possible solution, it doesn't address the actual problem. Firefox is still broken, and should be fixed rather than trying to get Microsoft to change anything, especially since it's already been established that Hotmail is not the only affected site.
Despite my thinking the pulldown menus was probably a separate issue and spinning off a separate bug, further investigation has revealed that this issue is also caused by the quick stubs changes and is related to the display property as well.  I just seems the patch here does not fix the pulldown menu issue.

Commenting out this line:

    'nsIDOMCSS2Properties.display',

in dom_quickstubs.qsconf avoids the issue.
Is this going to be fixed any time soon?
Yes.
Ok, thanks.
it's blocking 1.9.1 so it will be fixed til the final release of 3.1!
Attached patch patch v1.99 (obsolete) — — Splinter Review 
This got so much uglier :-/.

Major changes between this patch and the last one:
  * We now always create both a getter and a setter if need be (to avoid extra work later).
  * We reify the getter and setter for __define[GS]etter__ too (this was the reason that the options dropdown and help didn't work).
  * I had to export obj_define[GS]etter in order to avoid duplicating it (this also lets us get the error messages correct). Brendan, is there a better way?

This isn't v2 yet because I need to add more tests.
Attachment #353751 - Attachment is obsolete: true
Attachment #357060 - Flags: superreview?(brendan)
Attachment #357060 - Flags: review?(jorendorff)
(In reply to comment #78)
> Created an attachment (id=357060) [details]
> patch v1.99

With this patch, hotmail seems to work much better.  So far everything I have tried works including the dropdown menus.
I don't understand this.  Although the dropdown menus seem to work fine in my Linux builds, they still do not work in my windows builds.  I don't quite get what would be platform dependent here.
Bill, if you replace the assertion that reads:
+    NS_ASSERTION((jsval(pop) & 1) == 0,
+                 "Misaligned function pointer, crashes to follow");

to:
+    if(jsval(pop) & 1)
+    {
+        JS_ReportError(cx, "Property op is not about to work");
+        return nsnull;
+    }

do you see that in your error console?
(In reply to comment #81)
> Bill, if you replace the assertion that reads:
> +    NS_ASSERTION((jsval(pop) & 1) == 0,
> +                 "Misaligned function pointer, crashes to follow");
> 
> to:
> +    if(jsval(pop) & 1)
> +    {
> +        JS_ReportError(cx, "Property op is not about to work");
> +        return nsnull;
> +    }
> 
> do you see that in your error console?

Turns out I must have screwed something up in the Windows build I did last night.  This is working fine in my build from today.

This patch is included in my Windows and Linux builds available here:

http://www.wg9s.com/mozilla/firefox/

if others want to test.
Not sure if anybody tried but if you drag any message in Hotmail with the latest Trunk Build Firefox crashes.

Let me know if this should be filed under another bug report.
Attachment #357060 - Flags: superreview?(brendan) → superreview+
Comment on attachment 357060 [details] [diff] [review]
patch v1.99

163:         getter = nsnull;

should be getterobj, and root both that and setterobj with a vec[2] tvr.

Prepend js_ to obj_*? Not likely to collide but namespace purity calls to us.

This wasn't as bad as I thought from all the build-up ;-). Still want jorendorff full review.

/be
I am not seeing the drag message crash with Bill's build that includes the patch installed?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090115 Minefield/3.2a1pre ID:20090115050414
Your User Agent is: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-AU; rv:1.9.2a1pre) Gecko/20090115 Minefield/3.2a1pre

My user-agent is the same as yours, tried it again and it still crashes. The only add-on I use is Java Quick Starter.
Guys, please file a new bug on the crash. Let's keep this bug about the JS getter/setter regresion.
Attached patch patch v1.995 (obsolete) — — Splinter Review 
I still need to add tests, but this updated patch is still ready for review.
Attachment #357060 - Attachment is obsolete: true
Attachment #357467 - Flags: superreview+
Attachment #357467 - Flags: review?(jorendorff)
Attachment #357060 - Flags: review?(jorendorff)
(In reply to comment #82)
> (In reply to comment #81)
> > Bill, if you replace the assertion that reads:
> > +    NS_ASSERTION((jsval(pop) & 1) == 0,
> > +                 "Misaligned function pointer, crashes to follow");
> > 
> > to:
> > +    if(jsval(pop) & 1)
> > +    {
> > +        JS_ReportError(cx, "Property op is not about to work");
> > +        return nsnull;
> > +    }
> > 
> > do you see that in your error console?
> 
> Turns out I must have screwed something up in the Windows build I did last
> night.  This is working fine in my build from today.
> 
> This patch is included in my Windows and Linux builds available here:
> 
> http://www.wg9s.com/mozilla/firefox/
> 
> if others want to test.

This issue returned with the Windows build I created last night.  I am now doing all builds with the above code included so that if this happens again, I can answer this question.  I have no idea what is going on here though.  I have a script I run that does the builds and it should be doing the same thing on each build.
The patch in comment #82 reveals I am hitting the ASSERTION case.  Also returning nsnull in that case, like the patch in comment #82 suggests makes the dropdowns work (for whatever that is worth).
I get the following 2 errors in the error console under Windows that I do not get in my Linux builds.

Error: Property op is not about to work
Source file: http://gfx6.hotmail.com/mail/13.2.0260.1209/cmpt0.js
Line: 1
 ----------
Error: a.__compatMethods is undefined
Source file: http://gfx6.hotmail.com/mail/13.2.0260.1209/cmpt0.js
Line: 1
Attached patch Fixing Bill's issue — — Splinter Review 
This doesn't rely on the alignment of function pointers.

On IRC, jorendorff mentioned an alternative approach: to delete the offending property and letting regular XPConnect property resolution do its thing (so just forwarding to the regular lookupGetter. Since I"ve written this patch already, I'm going to keep it, but I think that jorendorff's solution is better overall.
Attachment #357467 - Attachment is obsolete: true
Attachment #357512 - Flags: superreview+
Attachment #357512 - Flags: review?(jorendorff)
Attachment #357467 - Flags: review?(jorendorff)
Comment on attachment 357512 [details] [diff] [review]
Fixing Bill's issue

Phew.  Well, this is pretty gruesome, but let's take it and move on to other stuff.  I have mostly comments and a few nits.

>+PropertyOpForwarder(JSContext *cx, uintN argc, jsval *vp)
>[...]
>+    JS_SET_RVAL(cx, vp, JS_ARGV(cx, vp)[0]);
>+    return (*popp)(cx, obj, v, vp);

This confused me for a second--if we're forwarding to a getter for a SHARED property, *vp should be initialized to JSVAL_VOID.  But of course getting it right for setters is more important.  Quick stub getters never depend on the initial value of *vp anyway.

>+    JS_SetReservedSlot(cx, funobj, 1, id);

To be strictly correct here you would call JS_IdToValue, I guess.  Add the call if you judge it appropriate --if there are already plenty of places outside SpiderMonkey that don't bother with the conversion, one more won't hurt.

>+    jsval id = JS_ARGV(cx, vp)[0];

Please change this variable's name.  SpiderMonkey uses "id" only for jsids.

>+    const char *name = JSVAL_IS_STRING(id)
>+                       ? JS_GetStringBytes(JSVAL_TO_STRING(id))
>+                       : nsnull;

Is there an API missing that makes you have to do this?  It seems like you could just pass the JSString pointer around.

>+    // Since XPConnect doesn't use JSPropertyOps in any other contexts,
>+    // ensuring that we have an XPConnect prototype object ensures that
>+    // we are only going to expose quickstubbed properties to script.

OK, as far as I know this is true, but I'm no expert.

>+// XXX Hack! :-/

/-:

>+static JSBool
>+DefineGetterOrSetter(JSContext *cx, uintN argc, JSBool setGetter, jsval *vp)

Nit:  setGetter?  :)  If you like "defGetter" or "wantGetter" or "getter" better, feel free to change it.

>@@ -153,16 +420,37 @@ xpc_qsDefineQuickStubs(JSContext *cx, JS
>[...]
>+        if(!JS_DefineFunction(
>+               cx, proto, "__lookupGetter__",
>+               reinterpret_cast<JSNative>(SharedLookupGetter),
>+               1, JSFUN_FAST_NATIVE) ||
>+           !JS_DefineFunction(
>+               cx, proto, "__lookupSetter__",
>+               reinterpret_cast<JSNative>(SharedLookupSetter),
>+               1, JSFUN_FAST_NATIVE) || [...]

Why not JS_DefineFunctions here?

>+++ b/js/src/xpconnect/tests/mochitest/test_bug462428.html
>[...]
>+var getter = Object.prototype.__lookupGetter__.call(document, 'documentElement');
>+ok(getter === undefined, "Good ol' lookupGetter doesn't do anything weird");

I would prefer not to test for this, as it's different from the original behavior and seems incidental.

>+    var setter = Object.prototype.__lookupSetter__.call(document, 'title');
>+    ok(setter === undefined, "Good ol' lookupSeter doesn't do anything weird");

Same here; also there's a typo here, "lookupSeter".

Please add a test for the __define[GS]etter__ behavior.

r=me with the few requested changes.
Attachment #357512 - Flags: review?(jorendorff) → review+
(In reply to comment #82)
> (In reply to comment #81)
> > Bill, if you replace the assertion that reads:
> > +    NS_ASSERTION((jsval(pop) & 1) == 0,
> > +                 "Misaligned function pointer, crashes to follow");
> > 
> > to:
> > +    if(jsval(pop) & 1)
> > +    {
> > +        JS_ReportError(cx, "Property op is not about to work");
> > +        return nsnull;
> > +    }
> > 
> > do you see that in your error console?
> 
> Turns out I must have screwed something up in the Windows build I did last
> night.  This is working fine in my build from today.
> 
> This patch is included in my Windows and Linux builds available here:
> 
> http://www.wg9s.com/mozilla/firefox/
> 
> if others want to test.

Nice work, Bill & Blake.  Tested the latest FF 3.2a1pre nightly build from your site and  I can access my Hotmail messages.  Can you make a patch like this for FF 3.1b3pre and Seamonkey 2.0a3pre, Blake?  I not only use Firefox browsers but also use Seamonkey browsers.
(In reply to comment #96)

> site and  I can access my Hotmail messages.  Can you make a patch like this for
> FF 3.1b3pre and Seamonkey 2.0a3pre, Blake?  I not only use Firefox browsers but
> also use Seamonkey browsers.

This patch has been reviewed and is ready for check-in, so it should be landing on the trunk soon.  It is also marked as blocking-1.9.1, so it will be landing on the branch as well, hopefully in time for Firefox 3.1b3.  That should cover all the browsers you asked about.
http://hg.mozilla.org/mozilla-central/rev/4014cb930c34
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Depends on: 475185
Depends on: 475291
(In reply to comment #97)
> (In reply to comment #96)
> 
> > site and  I can access my Hotmail messages.  Can you make a patch like this for
> > FF 3.1b3pre and Seamonkey 2.0a3pre, Blake?  I not only use Firefox browsers but
> > also use Seamonkey browsers.
> 
> This patch has been reviewed and is ready for check-in, so it should be landing
> on the trunk soon.  It is also marked as blocking-1.9.1, so it will be landing
> on the branch as well, hopefully in time for Firefox 3.1b3.  That should cover
> all the browsers you asked about.

Unfortunately, the "patch" has NOT been fully implemented as I've just tested the latest nightly builds of FF 3.1b3pre, FF 3.2a1pre & SM 2.0a3pre on the Hotmail site.  When I click on any of the messages I currently have, these browsers simply do nothing, meaning the email message is NOT being accessed or viewed.  disappointed.

Still waiting for Mozilla to get their act together in FULLY implementing your patch, Bill & Blake.
erpman1, "Bill & Blake" are "Mozilla", it's silly implying that some faceless power-that-is stands in our way.

Blake committed to the mozilla-central hg repository, see comment 98. The first Firefox 3.2a1pre nightly build that pulled source after that commit should have included the fix. So what buildid are you testing for the 3.2a1pre Firefox?

Either you are testing a build pulled before that fix hit mozilla-central, or the fix is insufficient (in which case you can blame "Blake" if not "Bill" :-P -- but better to not blame anyone and just report results -- with buildids)

/be
(In reply to comment #100)
> Unfortunately, the "patch" has NOT been fully implemented as I've just tested
> the latest nightly builds of FF 3.1b3pre, FF 3.2a1pre & SM 2.0a3pre on the
> Hotmail site.  When I click on any of the messages I currently have, these
> browsers simply do nothing, meaning the email message is NOT being accessed or
> viewed.  disappointed.
> 
> Still waiting for Mozilla to get their act together in FULLY implementing your
> patch, Bill & Blake.

Works for me on Ubuntu 8.10 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090126 Minefield/3.2a1pre ID:20090126020940. But it's not a Windows problem since I tested it with Windows 7 (also with the 3.1a1pre nightly of January 26th. It works perfectly for me. Thanks a lot guys!
(In reply to comment #102)
> 
> Works for me on Ubuntu 8.10 with Mozilla/5.0 (X11; U; Linux i686; en-US;
> rv:1.9.2a1pre) Gecko/20090126 Minefield/3.2a1pre ID:20090126020940. But it's
> not a Windows problem since I tested it with Windows 7 (also with the 3.1a1pre
> nightly of January 26th. It works perfectly for me. Thanks a lot guys!

Ok.  spoke a little too soon.  Downloaded & tested the latest Minefield nightly build (3.2a1pre) and I can now access my Hotmail messages (YAY!).  However, I can NOT compose new messages nor reply to existing ones as the message box and text format controls are "grayed out" or disabled.  Workaround for that problem is to set "general.useragent.extra.firefox" from the [about:config] window of Minefield with a value of "Firefox/3.0.5" or something close to it.  Then restart Minefield and the message box & format controls will no longer be disabled in Hotmail.  The message box/text format control "grayed out" problem seems to be a Hotmail site problem as it seems to check on Mozilla Firefox's "user agent" value.

But the patch still has not been included in the latest Shiretoko and Seamonkey nightly builds, so I'll wait for those to be updated.
(In reply to comment #103)
> [....]
> However, I
> can NOT compose new messages nor reply to existing ones as the message box and
> text format controls are "grayed out" or disabled.  Workaround for that problem
> is to set "general.useragent.extra.firefox" from the [about:config] window of
> Minefield with a value of "Firefox/3.0.5" or something close to it.  Then
> restart Minefield and the message box & format controls will no longer be
> disabled in Hotmail.  The message box/text format control "grayed out" problem
> seems to be a Hotmail site problem as it seems to check on Mozilla Firefox's
> "user agent" value.
> [....]

That's true. I suppose it's a Hotmail problem, so I think we can't do anything against that. For that, see Comment #66 ..
Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090126 SeaMonkey/2.0a3pre

Build ID: 20090126003045

This is still present, clicking on a mail does nothing but show "loading" in the corner.
(In reply to comment #105)
> Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
> rv:1.9.1b3pre) Gecko/20090126 SeaMonkey/2.0a3pre
> 
> Build ID: 20090126003045
> 
> This is still present, clicking on a mail does nothing but show "loading" in
> the corner.

Is this patch already checked in on SeaMonkey 2.0a3pre? That could be the problem.
(In reply to comment #106)
> (In reply to comment #105)
> > Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
> > rv:1.9.1b3pre) Gecko/20090126 SeaMonkey/2.0a3pre
> > 
> > Build ID: 20090126003045
> > 
> > This is still present, clicking on a mail does nothing but show "loading" in
> > the corner.
> 
> Is this patch already checked in on SeaMonkey 2.0a3pre? That could be the
> problem.

As I undersood from comment #101, it should be.
(In reply to comment #105)
> Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
> rv:1.9.1b3pre) Gecko/20090126 SeaMonkey/2.0a3pre

Note the rv: here: 1.9.1 -- this patch has been checked into mozilla-central (aka 1.9.2) *not* the 1.9.1 branch. Please stop commenting; I'm going to check the patch into that branch today, and hotmail will be fixed tomorrow (or in hourlies after I comment here with the relevant checkins).
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090126 Minefield/3.2a1pre.

Will verify the fix on 1.9.1branch after its landed.
Status: RESOLVED → VERIFIED
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/53090b97eecc ... gentlemen, you may rev your verifying motors.
Keywords: fixed1.9.1
I think the fix still has not been landed on the "comm-central" branch, which means the bug is still there for "SeaMonkey 2.0a3pre" nightly builds.
(In reply to comment #111)
> I think the fix still has not been landed on the "comm-central" branch, which
> means the bug is still there for "SeaMonkey 2.0a3pre" nightly builds.

comm-central does not contain this code. It contains only code specific to Thunderbird/SeaMonkey. Those builds pull code from either mozilla-central or mozilla-1.9.1. I believe the current SeaMonkey builds pull from mozilla-1.9.1, so they will pick up this change from there.
Status: VERIFIED → RESOLVED
Closed: 16 years ago → 16 years ago
We have lots of docs around, but maybe not all as organized as new people need to find what's what. But see http://mxr.mozilla.org/ for links to cross-referenced source hypertext from all the active repositories. As Ted said, comm-central is not a branch, it's a separate repository for *different code*. We try not to duplicate JS engine code too much, you know! :-P

/be
Ted: did you intentionally unverify this?
Yep, bugzilla didn't even warn me. :-(
Status: RESOLVED → VERIFIED
(In reply to comment #104)

> 
> That's true. I suppose it's a Hotmail problem, so I think we can't do anything
> against that. For that, see Comment #66 ..

Then it would be best to write feedback to Hotmail about the message box disabled problem here:
http://cfm.v4.msn.com/IRTWEB/Feedback/Feedback.aspx?clientid=37&nodeid=316&PageSource=en-us&Version=M2

Even K-meleon users were also affected by the Hotmail message box grayed out problem.  See here on this K-meleon forum page:
http://kmeleon.sourceforge.net/forum/read.php?1,87949

But at least I've now downloaded the newest nightly builds for Shiretoko 3.1b3pre and SM 2.0a3pre and they seem to have included the fix for most of the Hotmail problems.  I can now access my Hotmail messages in these builds.
(In reply to comment #117)
> (In reply to comment #104)
> 
> > 
> > That's true. I suppose it's a Hotmail problem, so I think we can't do anything
> > against that. For that, see Comment #66 ..
> 
> Then it would be best to write feedback to Hotmail about the message box
> disabled problem here:
> http://cfm.v4.msn.com/IRTWEB/Feedback/Feedback.aspx?clientid=37&nodeid=316&PageSource=en-us&Version=M2
> 
> Even K-meleon users were also affected by the Hotmail message box grayed out
> problem.  See here on this K-meleon forum page:
> http://kmeleon.sourceforge.net/forum/read.php?1,87949
> 
> But at least I've now downloaded the newest nightly builds for Shiretoko
> 3.1b3pre and SM 2.0a3pre and they seem to have included the fix for most of the
> Hotmail problems.  I can now access my Hotmail messages in these builds.

Now I have created a new bug entry about this. Please see bug 475767 .
(In reply to comment #118)
> (In reply to comment #117)
> > (In reply to comment #104)
> > 
> > > 
> > > That's true. I suppose it's a Hotmail problem, so I think we can't do anything
> > > against that. For that, see Comment #66 ..
> > 
> > Then it would be best to write feedback to Hotmail about the message box
> > disabled problem here:
> > http://cfm.v4.msn.com/IRTWEB/Feedback/Feedback.aspx?clientid=37&nodeid=316&PageSource=en-us&Version=M2
> > 
> > Even K-meleon users were also affected by the Hotmail message box grayed out
> > problem.  See here on this K-meleon forum page:
> > http://kmeleon.sourceforge.net/forum/read.php?1,87949
> > 
> > But at least I've now downloaded the newest nightly builds for Shiretoko
> > 3.1b3pre and SM 2.0a3pre and they seem to have included the fix for most of the
> > Hotmail problems.  I can now access my Hotmail messages in these builds.
> 
> Now I have created a new bug entry about this. Please see bug 475767 .

Let's hope that microsoft "fixed" their hotmail bug soon.
(In reply to comment #110)
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/53090b97eecc ... gentlemen,
> you may rev your verifying motors.

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090129 Shiretoko/3.1b3pre.  Thanks bkap!
Keywords: fixed1.9.1 → verified1.9.1
Whiteboard: relnote
I'm using a nightly build that I downloaded at about 2:30PM EST, Feb 10, 2009, 3.1b3pre (Shiretoko).  I confirm also that Hotmail now works correctly (On MacOS 10.5.6).

NICE JOB!
it wont let me open a message in my inbox
i have ff 3.1b2
it works on ie 7
(In reply to comment #123)
> it wont let me open a message in my inbox
> i have ff 3.1b2
> it works on ie 7

Either wait for 3.1b3 or use the nightly Branch builds.
Turns out the nightly build has problems too.  If I reply to a message in hotmail, the text of the original message doesn't get quoted in the editor.  

However, if I use IE7 or Firefox 3.0.6, then the quoting works correctly.  So something is broken in the nightly build.
(In reply to comment #125)
> Turns out the nightly build has problems too.

But it sounds like they are different problems. In that case there should be a new bug with all the details.
(In reply to comment #125)
> Turns out the nightly build has problems too.  If I reply to a message in
> hotmail, the text of the original message doesn't get quoted in the editor.  
> 
> However, if I use IE7 or Firefox 3.0.6, then the quoting works correctly.  So
> something is broken in the nightly build.

Are the problems in the nightly build able to produce the same effects as what this bug produces? If not, I suggest that you should report the new bug to Bugzilla. Thank You
Target Milestone: mozilla1.9.1 → mozilla1.9.2a1
Keywords: common-issue+ → common-issue-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: