Every time I switch themes, the web page that was being displayed in the web browser disappears once the switch is complete. I need to hit reload to make the page appear again.

this is xp. i remember hearing about this, that the page gets reset to about:blank, but i'm not sure if a bug was ever filed on it. i'll look.

Sending to skinability. This is a theme switching bug.

Adding [nsbeta2+][8/2] since bug 43350, which depends on this, is also. 43350 is data loss (as is this, if you've typed in a form), so critical.

So what's going on on skin change? How can this be fixed?

The IFRAME in the browser, editor etc need to save their document, src and pres state (according to hyatt). Currently they do not. Dave says to come talk to him for more information. Reassigning to Rick Potts, who I was told would have more of an idea about this than me :)

This is a general problem with our product. If you change an iframe to display: none and then restore it, the src and scroll position are lost. Theoretically the presState could be used to store this info, although I worry about us clashing with session history (which also uses this mechanism). In addition, editor needs to save its document and reattach it. It should probably save its caret position and the fact that the iframe is in edit mode as well. Again, this is a fundamental architecture problem that would have been exposed if you changed an <htmlarea> to display: none and brought it back.

I noticed that reloading the page after switching themes now crashes the browser (build 2000071112). Is there a separate bug for that?

I think we should close all windows before switching themes, for beta2.

Rick, this may just be a subset of the problem described in bug #45663.

*** Bug 45936 has been marked as a duplicate of this bug. ***

need this for beta3

Is this bug resolvable now that the Switch Theme button brings up a warning dialog? BuildID 2000072608 WinNT and Win32

*** Bug 45313 has been marked as a duplicate of this bug. ***

this blocks a beta3+.

no, the dialog is not acceptable. It was a warning with bad text as a cheap hack for PR2 just to get people to save and close any active documents before switching.

Well, this is really hard. What should happen is that nsHTMLFrameInnerFrame implements nsIStatefulFrame, and save/restore its docShell. With some jiggery-pokery, I can get this to happen, but unfortunately nothing draws -- the widget/view system gets messed up when you try to reinsert the old docShell, and nothing draws. I have a feeling that this _could_ work, but needs some input from view/widget folks. This solution has the advantage in that is works for both the browser and editor. The alternative is to just remember which URL the docShell was pointing at, and reload it. This is more do-able, but will screw editor/mail compose. Note that the alternative solution for editor would be to have a new docShell made, then call SetDocument() on it. Unfortunately, nsIDocShell::SetDocument() went away.

*** Bug 50783 has been marked as a duplicate of this bug. ***

*** Bug 51008 has been marked as a duplicate of this bug. ***

This bug (and 43350) were discussed in skins triage meeting - our opinion is that these two are data loss bugs and really should be P1. Are we going to be able to fix by 9/14?

You need to ensure that resources are committed for this bug. AFAICT, that is not the case.

Come on, you need to fix this. Just grab a bunch of people, and focus on only this bug for a day or two. (Yes, you'll have to slip the branch date back, but I think that should already be an obvious conclusion to everyone). Why did nsIDocShell::SetDocument go away?

Reassign to trudelle for triage.

This seems to fit the nsbeta3 bug criteria for priority P3: easily recoverable data loss (although the only data that is really *lost* is the session history), or obscure data loss, with an obvious workaround. P3 bugs are no longer plussable, I have been instructed by PDT to minus them, and completely agree in this case. nsbeta3-/future. Feel free to escalate to PDT if you disagree.

That leaves the dependent composer bug 43350 dead in the water. If you nsbeta3- this, you need to file a new bug to implement *something* to make skin switching acceptable, and to smoothly handle data loss in composer, mail compose, and AIM.

This bug isn't about any of those components, just the browser. As I said, if anyone disagrees, please escalate to PDT.

Why didn't we make this bug a priority? Do we want skin switching or not? If we do, then why can't we either choose a friday or a weekend and call it "Fix the bug with skin switching" day, and have everyone possible scowering over just those parts of code. You can't tell me that we wouldn't be able to think of SOMETHING to make this work smoothly. This is an extremely visible bug, and if we are going to use themes to sell Navigator 6, then we need to find SOME way of fixing this bug. This needs to become a higher priority.

clearing nsbeta3- for retriage

Hyatt thinks there is a safe easy way to do this for browser windows that would dovetail nicely with the PDT preferred solution, which prompts the user to close all editing windows, but leaver a browser window to be skinned alive on Win32 & Linux.

blah. I guess that would work, although that still makes this *HIGHLY ADVERTISED FEATURE* much more painful to operate then I would like. I'm adding the "arch" keyword because it's becoming clear that we lack the proper architecture to handle this properly.

How about the option of not switching the chrome for open editor windows? [$chrome=skin ?] A simple warning: You have editor window(s) open, to protect your data we will not change their $chrome, new windows will receive the new $chrome. <continue> <cancel>

Nope, this is for the web page, which is lower priority (P3). Easy to do, not an architectural problem, just not worth doing now. nsbeta3-

Excuse me??? How is this "EASY", and not an architectual problem? Since you didn't seem to read it, let me cut and paste the most detailed earlier comment: ------- Additional Comments From Simon Fraser 2000-08-25 17:09 ------- Well, this is really hard. What should happen is that nsHTMLFrameInnerFrame implements nsIStatefulFrame, and save/restore its docShell. With some jiggery-pokery, I can get this to happen, but unfortunately nothing draws -- the widget/view system gets messed up when you try to reinsert the old docShell, and nothing draws. I have a feeling that this _could_ work, but needs some input from view/widget folks. This solution has the advantage in that is works for both the browser and editor. The alternative is to just remember which URL the docShell was pointing at, and reload it. This is more do-able, but will screw editor/mail compose. Note that the alternative solution for editor would be to have a new docShell made, then call SetDocument() on it. Unfortunately, nsIDocShell::SetDocument() went away. ------------------------------------------------------------------------- Gee, "nsIDocShell::SetDocument() went away" sure the HELL sounds like an architectual issue to me, doesn't it? "Easy to do, not an architectural problem, just not worth doing now." Just not worth doing now??? This is the *NUMBER ONE* UI issue portraining to themes. What's the point of doing ANY POLISH if you're going to leave a HUGE LUMP OF DOGSHIT (this bug) right in the middle of the project? Ok, I'm going to stop THIS comment right here, and post the rant in the next comment, so at least one comment will be taken semi-seriously.

Thank you for that thoroughly professional comment. There are several ways to fix this bug. Simon and I tried an original approach in the hopes that it would be easy. It wasn't, and even if it could be made to work, the problem of persisting state across a reframe exists anyway. All skin switching does is expose bugs in the reframe of the XUL <editor> and <iframe> elements. These bugs have to be fixed regardless of how we decided to try to hack skin switching. The solution that covers both cases involves the saving of state using our presstate system. This is what the presstate system was designed for, and is a simpler solution. The only real hangup here is that the <editor> tag in XUL doesn't cleanly encapsulate the concept of being an editor. If it did, we wouldn't have to worry about things like reinitializing the editor across reframes. This would happen automatically whenever the <editor> iframe was constructed. Finally, there is a simple solution that can at least persist the browser's current web page (which is after all the title of the bug). This fix is easy as peter said and would work for browser iframes only.

The following comment is a rant (because my life fucking SUCKS right now, ok?). Please ignore it if you are not in the correct state of mind to read a rant. Seriously, did Netscape make a special effort to recruit marketers from XEROX? Or are you deliberately TRYING to sabatoge the success of this browser? HARD REALITY POINT #1: Netscape is selling Themes as a MAJOR FEATURE! In the 30 seconds of bullshit advertising spots, I'm SURE that themes/skins are going to be mentioned more then once. You're EVEN RUNNING A FUCKING CONTEST around themes! AND IT IS A FEATURE (one of the ONLY FEATURES I might add), that it is VISABLY APPARENT TO THE USER that NN6 has, and Internet explorer DOES NOT HAVE. It is therefore a KILLER FEATURE, one that users MIGHT SWITCH FOR! HARD REALITY POINT #2: Right now, it is VERY DANGEROUS for the average user to switch skins. They have to worry about FUCKING DATA LOSS!!! (Name ONE other project/program that causes DATA LOSS when SWITCHING SKINS. Go ahead. I DARE YOU.) HARD REALITY POINT #3: Even if the data loss problems GET FIXED (which is getting VERY unlikely because the PDT has its head up its collective ass when it comes to "avoidable data loss". (HEY PDT! DO *YOU* EXPECT *ANY* program to LOSE DATA when you're SWITCHING SKINS??)), switching skins is still a PAIN to the average user. ("HEY!!! Why did the web page that I was just on FUCKING DISAPPEAR? How do I get it back??" *Spends a few seconds fumbling around* "Oh, there it is. Whew. I'M NOT GOING TO DO THAT AGAIN." (emphesis added by me)). Switching skins is NOT an intellectually rigourous thing to do, so WHY SHOULD IT BE SUCH A PAIN to the user? If it's painful, USERS ARE NOT GOING TO USE IT! So in summary: You're taking what could be a KILLER FEATURE (*AND* a justification for the XPFE (which DOES make this browser slower on lower platforms)), and making it SO painful and dangerous that an AVERAGE USER WILL *NOT* WANT TO USE IT! That's just FUCKING GREAT. Could you make a WORSE MARKETING DECISION? No matter HOW HARD us programmers WANT to save Netscape's ailing reputation and market share, WE CANNOT DO IT IF YOU MAKE BONEHEADED OR DELIBERATELY MALICIOUS DECISIONS! We CANNOT stop you from ripping your guts out with a shipping knife, climbing up to the top of (one of the) Netscape building(s) with your last remaining ounce of strength and throwing them in Microsoft's direction. And even **IF** I stayed up ALL NIGHT for the next WEEK and created a patch that would FIX *ALL* of these problems, and actually make themes a VIABLE end-user feature again, YOU PROBABLY WOULDN'T ACCEPT THE PATCH, no matter *HOW* well written it was and well tested, citing "risk". Ever THOUGHT that it might be riskier *NOT* to fix things?? Ok, end of rant. I'm fucking tired of bashing my head against a brick wall.

Ok, this comment is NOT a rant (and I wrote the rant while Hyatt was writing his comment, so I hadn't read his comment when writing the rant). What bug#'s were filed about the reframe in the XUL editor? are any of them nsbeta3's? Can we PLEASE at least check in the fix that persists the current web page? (I think I've stated why we need that check-in in not so eloquent language.)

Your outburst isn't helping anything. No one in this bug is going to mark it nsbeta3++; you need to email pdt@netscape.com with clear, understandable reasons for why this needs to be fixed, and/or why it should be escalated. Hint: don't say `fuck' every other word. If they nsbeta3- it again, that's it - - end of story. You're welcome to take a crack at the bug yourself; I've seen you fix some other bugs, so I know you're capable of programming. I was especially confused by this comment: "No matter HOW HARD us programmers WANT to save Netscape's ailing reputation and market share, WE CANNOT DO IT IF YOU MAKE BONEHEADED OR DELIBERATELY MALICIOUS DECISIONS!" You are not a Netscape employee. As a Mozilla contributor, you can feel absolutely free to fix this bug, and if the patch is safe, it will be accepted. It is silly to say things like "XPFE makes the browser slower" because you're not only doing nothing to justify your cause, but you're making a fool of yourself. >Ever THOUGHT that it might be riskier *NOT* to fix things?? Have you ever thought that if Netscape delays its release to fix all bugs like this, there might not BE a Netscape anymore? Surely that is riskier, is it not? What you may not (apparently don't) realize is that Netscape is in the final stretch before RTM. There are surely hundreds of bugs like this that someone feels very strongly about. When bugs are considered by the PDT and managers, many factors are taken into consideration: Has anyone started work on this yet? (no). Is there enough time to fix this? (no, certainly not by beta3 anyways). Is there potential risk involved? (yes, probably) You yourself said that this fix will require some architecture reworking, and if that is indeed true, architecture changes can sometimes involve a lot of risk. Please settle any remaining issues you might have with pdt@netscape.com. I don't think anyone else here cares to hear your misguided complaints.

"Your outburst isn't helping anything. No one in this bug is going to mark it nsbeta3++; you need to email pdt@netscape.com with clear, understandable reasons for why this needs to be fixed, and/or why it should be escalated. Hint: don't say `fuck' every other word." That comment was a rant (as I clearly warned everybody). I would never send such a thing as a "formal proposal". A rant is a pure cry of frustration and the last (and pureist) form of a "wake up call". The only thing I guess it could help is make it clear how frustrated certain people are about this bug (besides just being a release of stress.). The PDT has already made it clear that they are conservative to the point of being reactionary, and they'd rather take the bug than take the "risk" of the change to fix the bug, even if the bug is a typo. Unless someone else higher in the structure hears the wakeup call, and overrides the PDT, they'll never agree to fix this bug. "I was especially confused by this comment: "No matter HOW HARD us programmers WANT to save Netscape's ailing reputation and market share, WE CANNOT DO IT IF YOU MAKE BONEHEADED OR DELIBERATELY MALICIOUS DECISIONS!" You are not a Netscape employee. As a Mozilla contributor, you can feel absolutely free to fix this bug, and if the patch is safe, it will be accepted." Oh, no it won't. I am absolutely free to fix this bug on my side, so skin-switching will be painless on MY computer, but if I can't even get the second part of 52154 checked in (which is a small, virtually riskless change), then how the hell can I expect to get a checkin that makes a lot of changes to xul renderors, etc, checked in? "It is silly to say things like "XPFE makes the browser slower" because you're not only doing nothing to justify your cause, but you're making a fool of yourself." You can't seriously be arguing that XPFE is FASTER or the same speed as a native user-interface? Lets see: Native: Direct calls to the GDI, or Code->GTK->X Server. XPFE: Load XUL file->Parse XUL file->Interpret XUL file->Layout XUL file->Render XUL file(Gecko)->GDI. (I'm sure I missed a few steps) It's never been a serious question about whether XPFE would be slower then a native interface, the question has always been whether the speed difference would be NOTICABLE and/or PAINFUL to the user. On my 666 MHZ pentium III, the speed of mozilla vs the native interfaces is perceptably the same. On my 233 MHZ pentium II at home, there is a slight perceptive difference. On a pentium 90, the speed difference is obvious. Skins/Themes are one of the ways that we can justify the difference in speed to the end user (along with other displays of flexability). "Have you ever thought that if Netscape delays its release to fix all bugs like this, there might not BE a Netscape anymore?" Bugs that MAJORLY impact specifically advertised functionality, that are 100% reproducable when trying to use that feature, that cause data loss to the user, and make use of that functionality generally painful and dangerous? How many OTHER bugs do you have that fit all of these criteria? If there are "Hundreds" of other bugs like this, then we shouldn't be thinking of shipping it at all, because no one would want to use it! But there AREN'T hundreds of these bugs (I use Mozilla as dogfood, so I know). This is the most obvious user-interface bug that I can think of in the entire project. The Skins people also think that way, they have asked repeatedly for this to be bumped up to P1. If we aren't going to fix these kinds of bugs, then why not ship today, right now, and get it over with? I'm also being told that there is an "easy" fix for this that does NOT involve architectual changes, but they refuse to check THAT change in, because of "risk".

Yes, it was a rant, and rants do not belong in bug reports -- they belong in newsgroups where everyone can feel free to ignore them at will. PDT oversees the project as a whole, whereas you are looking at a specific aspect of it, and thus they are far more qualified than you to say how important this bug is in the scheme of things. Yes, you can fix the bug on your side. Note that I said if the patch was *safe*, it would be accepted. If it causes more problems, it will certainly not be accepted, and that is understandable. I don't understand the issue with bug 52154. If you have a both a review and a super review from the module owner (the MODULE OWNER -- jag and akkana are not owners for that module), and have written reviewers@mozilla.org, then I will check the patch in. Let me know if you have actually gone through that procedure. I don't understand why XPFE slowdown is being brought up. Cross-platformness was a goal of Mozilla/Netscape6 from the start, and fixes are going in constantly to speed it up...if you used Mozilla builds as recently as 2 months ago, you know the major improvement in speed that's already been accomplished. You've lost me on the argument that "Skins/Themes are one of the ways that we can justify the difference in speed to the end user (along with other displays of flexability)." And who is `we'? Do you work for Netscape? Yes, there are many remaining dataloss bugs. This bug existed in PR2 with a warning dialog and I didn't see a single mention of it in PR2 reviews. Many of the remaining dataloss bugs are more important because they're not cases when we can just throw up a dialog. In this case, the dataloss is caused by something the user is voluntarily doing, and therefore it is easy for them to say "hey, this is telling me that I should finish my email message before switching skins. So I will." Furthermore, I expect that most people will switch themes from the browser, because it's the only part of the app that contains the handy View > Apply Theme menu. And the dataloss that occurs in the browser is extremely marginal (plus there's even a temporary fix being discussed). I disagree with your argument that the user won't figure out what to do if their page disappears...many will probably try reloading it instantly. I also can't see that many people wanting to switch skins, say, in the middle of composing an email. Lastly, since most people probably won't be switching skins all that often, it really shouldn't be too much of a hassle to comply with the instructions given in the warning dialog each time they want to. After they try to switch skins for the first time, they will learn to switch from the browser only with no composer windows open in the future. >This is the most obvious user-interface bug that I can think of in the entire project. Right. As I said before, your knowlege is limited to your use of the app; you do not have the knowledge of the project as a whole necessary to make decisions about arch-related bugs like this. "Fix the architecture so that this bug goes away" is not enough, you need to know and fully understand all possible repercussions. Anyways, I've said enough in this bug. Please move this discussion to the newsgroups and I will do the same.

Ok, last question (from me, anyways): Are we checking in the "easy" fix for this? If not, WHY not?

netscape engineers are not being allowed to work on anything less than p1 bugs. You can try to get it done by an external developer and checked in using mozilla rules.

hyatt: do you have even a partial patch for the easy fix? attach it and I'll finish it.

Dave Hyatt wrote: > This is a general problem with our product. If you change an iframe to > display: none and then restore it, the src and scroll position are lost. Are you collapsing and redisplaying the web page? Surely it's the one thing that won't be affected by the skin switch.

I have code for the simple browser fix.

If this code is checked in, then we need to change the suggested text for the warning dialogue (Bug 49579) - removing the part about reloading the web page.

Simon: Please attach the code for the fix?

simon: Pretty pretty pretty please?

Adding michaell@netscape.com (the themes marketing guy) to cc, since this bug will kill themes as a viable end-user feature for Netscape 6.

I have a complete fix in hand that stores everything for the browser. Session history, current URL, contents of form fields, etc. It's all good. Now I just have to lobby to get it landed.

I have a complete thoroughly tested fix for this bug in the browser window.

i've cleared the minus status to get this pushed into beta3. pdt, d'you agree? or do we wait till rtm? (i vote for beta3, imho.)

Hyatt: *please* attach the patch to this bug? Pretty Pretty Please? Is there any monitary incentive that I can give you to attach your patch to this bug? REASON: If this fix doesn't make it into nsbeta3/rtm, I would like to apply your fix to my personal copy of mozilla (and I'm sure other people would want to as well). Attaching your patch to this bug makes sure that it doesn't "disappear" in one way or another. Either that, or create an mp3 of the code (a-la-decss)?

cc'ing phil for his thoughts on ++'ing this. Jason, such requests for patches could easily be handled via email, without having to spam everyone cc'ed to this bug.

The data loss bugs with skin switching (bug 43350) should be rtm++. Losing the active browser page is not rtm++, IMO. I wouldn't *hold* nsbeta3 for either of those issues, but would take a safe fix for the data loss for nsbeta3 (although bug 43350 is currently nsbeta3-)

BTW, my opinion is only part of the call -- PDT isn't a one-man show. And we get argued out of opinions all the time by people who are closer to the issues than we are. It's a flexible process based on two-way feedback :-)

I know you can't make the call alone, I was just wondering what you thought about this. Hyatt, will your patch also fixed the typing problem in Composer after switching skins (see bug 43350)? If so, 43350 should be marked as a dup of this, and then this can be ++'ed.

Hyatt's changes won't work for editor or mail compose.

However, they WILL stop dataloss in session history, textarea's of the existing page, and the page data itself. The people who didn't complain about this bug in PR2 didn't complain because they thought we were going to FIX it before the final release (as implied by saying "themes are a prototype feature"). Everyone I have shown skin-switching to has said "You lose your current page when switching skins?? Isn't that very annoying?" I always answer "Yes, but we're going to FIX that."

It's obvious from the # of people cc'ed on the bug and from the huge # of comments that this is a wanted fix. I have been running with it for days now and it works great. It also lays the groundwork for editor/mailcompose to one day be fixed. I'd like to request that this bug be marked ++ by PDT.

pdt?

Dave, please attach patch to bug, and get super-review and any module owner review indicated in bug. At that point, I can rtm+. I don't think this qualifies for nsbeta3++, although you are welcome to plead the case to PDT if you feel strongly otherwise. If you do, please use email rather than waiting for the meeting. ->M19

cc jrgm

I don't think it is beta3 material either. I just want an rtm+.

Okay, but you won't get rtm++ until you've done as I asked.

Will get a super-review today.

I'd like to call your attention to bug 53838, an nsbeta3++ bug that needs to get resolved first.

*** Bug 54619 has been marked as a duplicate of this bug. ***

Hey Dave, your fixes for this bug introduced some leaks. We should review those diffs again...

Please remove 'need info' when you have attached patch, and the reviewers have put r=/a= in the bug report.

This fix was checked into the trunk on October 2nd, at 4:21 pm or so. (According to bonzai). Just adding this info because no comment was made about when it was checked in and it's a pain to search bonzai for specific checkins.

According to Tinderbox, this fix added 9kb's of leaks. Going to look through the diffs now.

The leak issue is bug 47763.

Sorry for the spam, but I want to clarify something: John: are you saying that the new TB leaks are unrelated to the fix for this bug? (as I hope you are saying). The bug that you linked to seems unrelated to the fix for this bug.

Yes, it's not immediately apparent (at least to me :-/) that these are related, but they are. See news://news.mozilla.org/39D97624.B830FB7B%40netscape.com

What's the story on this fix? How could it even get on the trunk without an attached patch and a=/r=? Is there some problem with the reviews?

Bumping priority to P2, since this is high profile.

It was super-reviewed by waterson.

Anyone have a patch to attach retroactively? Or just the bonsai link will do. /be

Here are the bonsai links for the original checkin on the trunk: http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/xul/base/src&command=DIFF_FRAMESET&file=nsPopupSetFrame.cpp&rev1=1.60&rev2=1.61&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/xul/base/src&command=DIFF_FRAMESET&file=nsMenuFrame.cpp&rev1=1.158&rev2=1.159&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/xul/base/src&command=DIFF_FRAMESET&file=nsBrowserBoxObject.cpp&rev1=1.2&rev2=1.3&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/xul/base/src&command=DIFF_FRAMESET&file=nsBoxObject.h&rev1=1.4&rev2=1.5&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/xul/base/src&command=DIFF_FRAMESET&file=nsBoxObject.cpp&rev1=1.4&rev2=1.5&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/xul/base/public&command=DIFF_FRAMESET&file=nsIBoxObject.idl&rev1=1.2&rev2=1.3&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/html/document/src&command=DIFF_FRAMESET&file=nsFrameFrame.cpp&rev1=3.128&rev2=3.129&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/base/src&command=DIFF_FRAMESET&file=nsPresState.cpp&rev1=3.8&rev2=3.9&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/base/public&command=DIFF_FRAMESET&file=nsIPresState.h&rev1=3.3&rev2=3.4&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/xpfe/global/resources/content&command=DIFF_FRAMESET&file=xulBindings.xml&rev1=1.36&rev2=1.37&root=/cvsroot http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/xpfe/global/resources/content&command=DIFF_FRAMESET&file=treeBindings.xml&rev1=1.41&rev2=1.42&root=/cvsroot There was also a checkin yesterday that had "fixing leaks created by 44437" in the checkin description, but I don't know if that was a modification to his patch, or if he was fixing the underlying architecture.

taking a closer look, the "leak" checkin was changes to xbl, not xul, so I would conclude that it was changes to the underlying architecture. This patch has worked PERFECTLY on my nightly build, and has increased stability during skin switching. It would really help the product if this change could get checked into the branch.

*** Bug 55310 has been marked as a duplicate of this bug. ***

What needs to be done to get this on the branch? I've given you the links to bonsai which is extremely similar to a patch. We also have a r=a=waterson to get this onto the trunk. Can this be changed to RTM+ now?

yeah, it seems ready.

PDT marking [rtm++]

From bonzai, it looks like 8 of the 10 patches for bug 44437 was checked into the branch right after this bug got RTM++'ed. (5:40 pm or so). Is the complete fix checked in now? :-)

Damn. I missed two files. Holding open until I get the last file checked in on the branch.

WFM, but is it impossible to avoid sending http request? The current page disappears yet on offline mode.

I found the message pane of mailer window disappears when switching themes.

This bug is about the web page, not Mail.

Is this patch in the latest nightly from this morning (I'm using Build ID: 2000100904)? Because I have found an interesting behavior not previously mentioned in the thread for this bug....which has exsisted for some time now. When I set the IFRAME style attribute "display:none" and then attempt to change the contents of the (hidden) IFRAME, it seems as though the IFRAME is no longer listed in the DOM frames array. The browser simply launches a new browser window with for the intended target page (as if the frame target requested does not exist). For Example, Filename: IframeTest.htm <HTML> <HEAD> <TITLE>Hidden IFRAME Test Page 1</TITLE> </HEAD> <BODY> <h1>&lt;IFRAME&gt; Sample Page</h1> The IFRAME is below this line:<br> <IFRAME NAME="Frame1" SRC="frame1.html" style="display:none"> </IFRAME><br> The IFRAME is above this line.<br> <hr> <a href="frame2.html" target="Frame1">Load another page into the IFRAME</a><p> <a href="frame1.html" target="Frame1">Reset the IFRAME</a> </BODY> </HTML> Filename: frame1.html <HTML> <HEAD> <TITLE>frame1.html</TITLE> </HEAD> <BODY> <h2>Hello World: This is the page frame1.htm</h2> <P>&nbsp;</P> </BODY> </HTML> Filename: frame2.html <HTML> <HEAD> <TITLE>frame2.html</TITLE> </HEAD> <BODY> <h2>Hello World: This is the page frame2.htm</h2> <P>&nbsp;</P> </BODY> </HTML> Note that when you change the style "display:none" to "display:block" it works as expected, frame2.html is loaded into the IFRAME Has anyone tested this scenario on this new patch that is being discussed?

Could people please refrain from adding comments to what is an already long bug, particularly when those comments have NOTHING to do with the bug at hand. As for the previous comment, one should review the meaning of 'display:none'. That does not mean it is hidden; it means that it is not there, at all. Ergo, what you describe is not a bug.

Hyatt: Did the last two files get checked in on the branch?

KOIKE Kazuhiko: you want bug 43350 which is the equivalent of this for mail.

What is the holdup (5 days to checkin 2 files) on this? This needs to land NOW.

Fixed everywhere.

*** Bug 56491 has been marked as a duplicate of this bug. ***

*** Bug 56491 has been marked as a duplicate of this bug. ***

Can someone provide a list of functionality that should now be fixed with this patch, since it seems to fix more than just the original problem (disappearing webpage), so QA can verify?

cc: pmac to verify this on the branch builds.

The fix seems to persist the current URL and scroll position. However it does not persist the current state of form elements, leading to data loss. Since this bug is getting so unwieldy, I filed a new bug (bug 56982) for that. Blake, you should also check whether changes made to the content of a page by scripts are persisted.

verifying on branch builds for 20001030am mac/linux/win32 -- current URL, session history, form field changes (radio/checkbox/text/textarea/select), scroll position, are all preserved on a skin switch in the browser. Marking vtrunk. Please file bugs for specific aspects of state that not preserved (noting that bug 43350 is already filed on mail/composer issues, and that as far as I can see bug 56982 for form fields is working fine for me mac/linux/win32).

Verified Fixed on Mozilla trunk builds linux 122710 RedHat 6.2 win32 122705 NT 4 mac 122708 Mac OS9 Setting bug to Verified and removing vtrunk keyword