Closed Bug 43350 Opened 24 years ago Closed 23 years ago

[embed]Composer / Mail Compose window contents are nuked when switching themes (skin switching)

Categories

(Core Graveyard :: Skinability, defect, P1)

Product:
Core Graveyard
Component:
Skinability
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla1.0

People

(Reporter: jeziorek, Assigned: hyatt)

References

Details

(Keywords: arch, embed, Whiteboard: [Hixie-P0] [behavior][nsbeta1+] UE1)

Attachments

(1 file)

crash stack after closing the nuked Compose window
6.50 KB, text/plain
Details 
1. Launch Mozilla Browser for build 2000-06-21-08-M17
2. Tasks | Composer
3. Edit | Preferences
4. Under the heading Appearance click on Themes
5. Click  Modern theme and apply it
6. Press OK
7. try and enter text into the body
It doesn't work and if you try to Debug | Insert Text at this point it crashes
Keywords: smoketest
I'm asking Paul Hangas and the theme team to take a look at this and see if it 
may be something from their end.
Assignee: beppe → hangas
Sending to owner of Skinability for a crack at this...
Assignee: hangas → ben
Component: Editor → Skinability
QA Contact: sujay → BlakeR1234
Yup, I notice this also with 2000062108 winME/win2K.  There's the same problem 
if, after following the instructions that Alek gave, you open MailNews and try 
to type in the body of a new message (open MailNews, press New Msg, and try to 
type in the body...doesn't work).  Would split that into another bug but I 
assume they're closely related.
OS: Linux → All
Hardware: PC → All
adding myself to cc: list.

nominate for nsbeta2 since I assume we'll encourage people to try out themes and 
use mail/composer at the same time :-)  Being unable to type in the composer 
area is not good.
Severity: normal → major
Keywords: crash, nsbeta2
Adding kin@netscape.com to cc list.
Putting on [nsbeta2+] radar for beta2 fix.
Whiteboard: [nsbeta2+]
Somebody explain to me how this can be a smoketest blocker.  I don't think so.
Priority: P3 → P1
Target Milestone: --- → M17
Ben, is this even our bug?  Or something for hyatt and co.?
Target Milestone: M17 → M18
I also fail to see how this is a smoketest blocker.

Alek added it, and I assume he was referring to this test:
E.2 Insert text In the new Editor window, select Debug | Insert Text 

However, smoketesting does not require that you change to the modern theme 
prior to that, afaik.

Removing smoketest kw, Alek if you readd it please give a good reason why...
Keywords: smoketest
Blake, you rule!  I was just typing in the same thing. :-)

This should probably go to me. It sounds hard to fix, tho.
Assignee: ben → sfraser
*** Bug 43252 has been marked as a duplicate of this bug. ***
added fix by date in status field
Whiteboard: [nsbeta2+] → [nsbeta2+][8/2]
setting to m17
Target Milestone: M18 → M17
*** Bug 43646 has been marked as a duplicate of this bug. ***
This is most likely a dup of 44437. Since any web content gets nuked on a skin 
change, of course we're gonna lose the editor content also.

This is data loss, hence critical.
Severity: major → critical
Status: NEW → ASSIGNED
Depends on: 44437
No longer depends on: 44437
Adjust summary
Summary: Changing to a modern theme and then trying to type in the body doesn't work → Composer contents are nuked and can't type after switching themes
Is is possible to not allow theme switching when Composer is enabled?  We would 
prefer this for beta2.

beppe, reminder to try this with Mail compose.
I'd like to point out that in addition to losing Composer data and the 
current web page (as noted in 44437), you also lose the history list AND 
and Instant Messenger conversation logs.  Basically, all state and history 
data is gone.
I think we should close all windows before switching themes, for beta2.
Depends on: 44437
it is very unlikely that this will get fixed for beta2. It depends on some 
infrasctuture that is also needed in the browser (bug 44437). As I said in 44437, 
I think we should code a workaround that closes all windows (prompting to save in 
composer) before the theme switch. That should be coded as a temporary fix for 
44437.
Whiteboard: [nsbeta2+][8/2] → [nsbeta2+]
Well, 44437 got nsbeta2-. I think, for beta2, we need to put up dialog warning 
the user that switching themes will blow away all their window contents (what 
happens to mail????). Ben should do that, so here's a bug, dude.
Assignee: sfraser → ben
Status: ASSIGNED → NEW
Summary: Composer contents are nuked and can't type after switching themes → Need to warn user about data loss before switching themes
bug 45936 describes one consequence of mail
Depends on: 45936
cc vera for ideas on what text the warning dialog should include
Whiteboard: [nsbeta2+] → [nsbeta2+] fix in hand.
I have a fix for this. Note that I've made the text really really bad so that 
we're all so ashamed of ourselves that we make fixing this problem the right way 
a priority for nsbeta3 or FCS.
Status: NEW → ASSIGNED
checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Yup, the text is awful (nice job).  I do suggest, however, that you change 
Editor to Composer, as most people probably don't consider Composer windows to 
be Editor windows.
Erm, that last post was from me but Bugzilla was remembering my friend's login.

In any case, looking back, this bug got completely morphed from the original 
Composer issue(s) to the request to throw a warning dialog.  I think we need to 
either reopen this, mark it nsbeta2-, and change the summary back to the 
original problem, or file a separate bug for the original issue...
Yes, please give me this bug back as the composer data loss issue.
Status: RESOLVED → REOPENED
Keywords: crash, nsbeta2 → correctness, nsbeta3
Resolution: FIXED → ---
Summary: Need to warn user about data loss before switching themes → Composer window contents are nuked when switching themes
Whiteboard: [nsbeta2+] fix in hand.
Target Milestone: M17 → M18
Giving back to self.
Assignee: ben → sfraser
Status: REOPENED → NEW
setting to nsbeta3+
Whiteboard: nsbeta3+
setting priority in status whiteboard - high risk!  may need help from 
XUL/Layout folks
Priority: P1 → P3
Whiteboard: nsbeta3+ → [nsbeta3+][p:3]
Keywords: UE1
Blocks: 49759
*** Bug 47216 has been marked as a duplicate of this bug. ***
waiting for 44437
Whiteboard: [nsbeta3+][p:3] → [nsbeta3+][p:3][BLOCKED]
moving this out to m19, 44437 is still open, odds of that getting in and time 
for this one to be fixed and tested before b3 is not realistic
Whiteboard: [nsbeta3+][p:3][BLOCKED] → [nsbeta3-][p:3][BLOCKED]
Target Milestone: M18 → M19
What? Is this really nsbeta3- ?
sorry, i missed your comment.  definitely need this for RTM, then...
Keywords: rtm
Discussed this bug in skins triage meeting and felt like this should be a P1 bug
because it involves data loss.  Are we not going to be able to fix this (and
44437) for PR3?
The bug that this is dependent on, 44437 has been futured.  Do the same to this 
one and you may as well kiss skin switching goodbye...
Summary: Composer window contents are nuked when switching themes → Composer / Mail Compose window contents are nuked when switching themes
Why does this depend on bug 44437? Isn't this an editor data loss problem? 
Won't each component have to deal with preserving its own data on a dynamic 
theme switch?  The browser's "data loss" issues seem quite separate, and are 
lower priority.  Perhaps there is some common work needed for both, such as 
closing all windows, for which no bug has been filed? 
This bug depends on 44437 because they should share a common solution. The 
editor, after all, is pretty much a browser window holding a document upon which 
an editor has been created.

For this to be fixed for editor, some architecture work needs to be done which is 
*exactly* what the browser would use to preserve its contents. It would be 
impossible, or very hard, to come up with a scheme for editor windows to preserve 
their contents which does allow the browser to do the same thing (i.e. what *has* 
to be done for editor would also fix browser).

This comment applies to bug 44437 as well.  Can we implement the work-around
solution that Simon suggested, namely:

1)  Pop up a warning dialogue when the user attempts to switch skins telling
them to close any open windows (or just specifically the affected windows -
composer, new message, etc.) before switching and

2)  Remember and reload the current URL so that the web page is not lost, or at
a minimum tell the user to reload the page after the switch in the dialogue
mentioned in 1).

Can we do this?  Obviously fixing this is the best solution, but doing 1) and 2)
would make the user experience much more agreeable in the interim.
Why didn't we make this bug a priority? Do we want skin switching or not?

this should be rtm++

Is this "rtm++" as to:
  Preserve the data as we switch skins? or
  Warn the user that data will be lost?

I think it's the latter, but I just want to be sure.

If the latter, then that's a different bug - Bug 49759.  Hyatt seems to have a
fix for 44437 (lost web page in browser when switching skins), I don't know if
that means we're any closer to fixing this or not.

Oh, ok, so this bug represents preserving the data across a skin switch? In that
case, I retract my rtm++ statement, since I understand that to be infeasible for
seamonkey.

*** Bug 53795 has been marked as a duplicate of this bug. ***
ok stick with me on this one -- it gets confusing, there are 6 bugs involved in 
the themes switching thing. 

The big '2' are 44437, which is owned by hyatt and this one owned by sfraser. 
Hyatt has the fix for everything but composer. Bug 45936 was duped against 44437 
(great), bug 49759 is also blocked by 44437.

This bug is also blocking 49759. However, this bug is also being blocked by 
44437. Are you confused yet? Then 53795 was duped against 43795, which has been 
duped against this bug. Now, 43795 was the bug that talked about throwing up the 
dialog to close windows before changing the theme. There is a js patch attached 
to that bug that would do that, but it will need to be reviewed and tested.

At this stage, I recommend that we not deal with trying to preserve the data, it 
is more than acceptable to ask the user to please save any exisitng files and 
then select apply or whatever the dialog asks them to do. This type of request 
is not new by any means. 

Bottom line: PDT please mark this bug rtm++, this will unblock 49759, which by 
the way is yet another bug dealing with the warning dialog -- so, shouldn't that 
really be a dup of this bug?
Priority: P3 → P1
Whiteboard: [nsbeta3-][p:3][BLOCKED] → [nsbeta3-][p:3][BLOCKED][rtm+]
i believe the themes switching issue is being resolved by bug 49759, marking 
this future, this will resolve the issue by a more in-depth resolution which can 
wait till post rtm. unblocking 49759
No longer blocks: 49759
Whiteboard: [nsbeta3-][p:3][BLOCKED][rtm+] → [nsbeta3-][p:3][BLOCKED][rtm-]
Target Milestone: M19 → Future
*** Bug 55579 has been marked as a duplicate of this bug. ***
Add ChatZilla to the list of components that get "nuked" when skins are switched.

Oh, and there is no warning when you switch skins from the view menu.

Jake

*** Bug 56871 has been marked as a duplicate of this bug. ***
URL: Composer / Mail Compose window conten...
QA Contact: blakeross
Summary: Composer / Mail Compose window contents are nuked when switching themes → [nsbeta3-][p:3][BLOCKED][rtm-]
MPT you just toasted the bug fields. what app were you using?
[trying to reset]; cc endico
URL: Composer / Mail Compose window conten...
Summary: [nsbeta3-][p:3][BLOCKED][rtm-] → Composer / Mail Compose window contents are nuked when switching themes
Uhm, hopefully that was bug 56982, and not something more sinister. Sorry. 
Restoring QA contact, too.

(That would perhaps also explain why Bugzilla asked me earlier if I wanted to 
mark a bug as a duplicate of Ben Goodger ...)
QA Contact: blakeross
Keywords: nsbeta3, rtm → nsbeta1
Whiteboard: [nsbeta3-][p:3][BLOCKED][rtm-]
bug 44437 has been fixed. Would be cool to get skin switching right.
*** Bug 61364 has been marked as a duplicate of this bug. ***
moving to mozilla0.9
Whiteboard: [tracking]
Target Milestone: Future → mozilla0.9
Blocks: 59374
*** Bug 66353 has been marked as a duplicate of this bug. ***
updating summary, whiteboard and keywords
Keywords: embed
Summary: Composer / Mail Compose window contents are nuked when switching themes → [embed]Composer / Mail Compose window contents are nuked when switching themes
Whiteboard: [tracking]
*** Bug 54397 has been marked as a duplicate of this bug. ***
transferring status from 54397, but beppe can feel free to minus (since assigned 
to editor engineer).  cc'ing mscott
Keywords: mail3
Whiteboard: [nsbeta1+]
When the nuked compose window is closed, crash occurs in Mac builds.
I also see the crash with a simliar stack trace using a mozilla 02/06 build on 
win32. This particular crash is in nsEditorShell.cpp line 331. Basically, it's a 
dangling pointer in mContentAreaDocShell. mContentAreaDocShell itself isn't 
null, but as I saw on a mac debug build, the memory block had already been 
freed. This particular crash is making it difficult to verify some skinnability 
bugs that I'm trying to triage because I keep running in to it.
Adding myself to cc list
*** Bug 71236 has been marked as a duplicate of this bug. ***
Moving to 0.91. Fixing this bug needs some heavy lifting by someone intimate with 
docShell, documents etc. ccing docShell people. What needs implementing here is 
nsDocShell::SetDocument(), which used to be there, but was removed.
Target Milestone: mozilla0.9 → mozilla0.9.1
Now this bug seems to cause a crash in bug 72076.
This also affects AIM.
*** Bug 71129 has been marked as a duplicate of this bug. ***
We need the theme switching dialog back, bug 75634 filed.

Temporarily dependent on bug 75634.
Depends on: 75634
*** Bug 63820 has been marked as a duplicate of this bug. ***
CNET complained about this in their initial 6.0 review:

"One big complaint about themes: you have to close all nonbrowser Netscape 
windows, such as email, before switching themes. Otherwise, you'll lose any 
unsaved work from those windows."

This probably shouldn't slip by another release cycle...if docshell work is 
needed to start, let's get the appropriate people involved (cc'ing rpotts, 
valeski, adamlock).
Keywords: arch, mozilla0.9.1, nsCatFood
Also removing dependency on 75634.  That bug will mask this problem, but 
doesn't need to be fixed to go ahead with this one (and it's creating a false 
sense of having to close out other bugs before fixing this).
No longer depends on: 75634
Keywords: mostfreq
Blocks: 68973
Adding this as a depends bug to 68973, META skin switching tracking bug.
*** Bug 77929 has been marked as a duplicate of this bug. ***
In 050208 trunk build, Mail message pane is also nuked, and causes crash. This
can not be prevented by 75634. I have not tested in 0.9 builds, but it is
potentially a blocker if it happens.
Blocks: 77235
moving out to 1.0, to get this resolved we will need help from 
rpotts/adamlock/hyatt/maybe valeski
Target Milestone: mozilla0.9.1 → mozilla1.0
also, please review bug 44437 for detailed information on what needs to get 
done, to quickly summarize:

"we need to take an existing, in-memory document, and set it on a docShell for 
this to work (this is a New Thing). Currently, all documents are created by 
loading via necko so I expect there are lots of issues that will need working 
out to make this work properly"
If we're pushing this off, we need to reimplement the themes switch warning dialog.
tpringle: bug 75634 exists for the theme switching dialog.
Whiteboard: [nsbeta1+] → [behavior][nsbeta1+]
*** Bug 72328 has been marked as a duplicate of this bug. ***
See also bug 83792, browser content area is nuked and the page is reloaded from 
the server on theme switch.
Keywords: UE1
Whiteboard: [behavior][nsbeta1+] → [behavior][nsbeta1+] UE1
Summary: [embed]Composer / Mail Compose window contents are nuked when switching themes → [embed]Composer / Mail Compose window contents are nuked when switching themes (skin switching)
Whiteboard: [behavior][nsbeta1+] UE1 → [Hixie-P0] [behavior][nsbeta1+] UE1
Blocks: 104166
Build: 2001110503..... hell even 0.9.4
Since the the theme-change wont occur until you restart mozilla this can not be 
reproduced!!! So i'm setting it as invalid...
May it's a fix... but i prefer not to restart mozilla to change a theme.
Status: NEW → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → INVALID
Reopening; this bug is still present it is just masked by our current behaviour
of requiring a restart (which is itself a bug).
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
don't move bugs that are in the 1.0 dependency tree. sorry.
Target Milestone: mozilla1.0.1 → mozilla1.0
-> mjudge
Assignee: sfraser → mjudge
Status: REOPENED → NEW
--> mine.
Assignee: mjudge → hyatt
Fixed.  <editor>s now retain all of their contents across a skin switch.
Status: NEW → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: