Closed
Bug 32148
Opened 25 years ago
Closed 17 years ago
window maximized state not persisted
Categories
(Core :: XUL, defect, P3)
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: danm.moz, Unassigned)
References
Details
Attachments
(10 files)
4.31 KB,
patch
|
Details | Diff | Splinter Review | |
6.10 KB,
patch
|
Details | Diff | Splinter Review | |
10.23 KB,
patch
|
Details | Diff | Splinter Review | |
70.80 KB,
patch
|
Details | Diff | Splinter Review | |
231.10 KB,
patch
|
Details | Diff | Splinter Review | |
67.80 KB,
patch
|
Details | Diff | Splinter Review | |
62.24 KB,
application/zip
|
Details | |
22.86 KB,
image/jpeg
|
Details | |
9.05 KB,
patch
|
Details | Diff | Splinter Review | |
8.13 KB,
patch
|
Details | Diff | Splinter Review |
(Just a reminder:) Maximize a browser window (or almost any window in the
product) and close it. The next (browser) window opened will be in the correct
size and location of the pre-maximized window, but it will not be maximized.
Better 'twere maximized.
Note we won't be able to do this on gtk, where we can't even tell that we're
maximized. But Windows and the Mac are able to track that and restore in a normal
or maximized state.
Comment 2•24 years ago
|
||
mass-moving all bugs to m21 that are not dofood+, or nsbeta2+
Target Milestone: M18 → M21
Comment 3•24 years ago
|
||
This is a real annoyance on Windows. Also (and perhaps more importantly?), if I
maximize a window and then open second mozilla window, it's not maximized, so
I'm constantly having to hit the maximize button in the titlebar. Very
annoying.
Moving to M21 from future as per comments from Peter Trudelle. I think this one
needs to be fixed to release a polished product.
Comment 4•24 years ago
|
||
no, this one is now future. We will have to leave this much polish till a
future release.
Target Milestone: M21 → Future
Comment 5•24 years ago
|
||
Peter Trudelle, is this so hard to implement? Many people are suffering from
this bug... I think it must be in release!
I agree, please take my frown face seriously when I say that this is one of the
bugs that I most want fixed. :(
Comment 7•24 years ago
|
||
I'm suffering too, but we simply do not have time for this level of bugfix. We
could possibly still take a good patch though. Adding helpwanted keyword. :`(
Keywords: helpwanted
Added dependency on bug 30394 because before having persistent inheritage,
we must have non-persistent inheritage, which is available in JavaScript via
window.open() parameter (Navigator 3 and 4 behavior/compatibility).
Depends on: 30394
Comment 10•24 years ago
|
||
*** Bug 41543 has been marked as a duplicate of this bug. ***
Comment 11•24 years ago
|
||
Sorry 'bout the spam... having much trouble with Bugzilla. Personally, I don't
think Mozilla should open up in the minimized state, in which case the last
restored state should be persisted. i.e.- maximized or last window position and
size.
Comment 12•24 years ago
|
||
Would this just be a dupe of bug 20847, rather than depending on it?
Comment 13•24 years ago
|
||
I would add my vote for this, but I don't have any more left ... :(
Comment 14•24 years ago
|
||
Nominating for RTM. This is driving me nuts being a laptop user.
Keywords: rtm
Comment 15•24 years ago
|
||
cc self
Comment 16•24 years ago
|
||
*** Bug 42588 has been marked as a duplicate of this bug. ***
Comment 17•24 years ago
|
||
*** Bug 42588 has been marked as a duplicate of this bug. ***
Comment 19•24 years ago
|
||
Adding myself hidday@geocities.com to CC.
It seems there are alot of small bugs related to the
minimizing/maximizing/window restoring on Win32.
Comment 20•24 years ago
|
||
adding to CC: mdaskalo@tlogica.com
This is something very annoing.
Reporter | ||
Comment 21•24 years ago
|
||
Keywords: helpwanted
Whiteboard: [rtm-] → [rtm need info] patch attached
Comment 23•24 years ago
|
||
Is this technology available only on Windows? Other nits:
+ if (sizeString.EqualsWithConversion("m"))
+ mSizeMode = nsSizeMode_Minimized;
+ if (sizeString.EqualsWithConversion("M"))
+ mSizeMode = nsSizeMode_Maximized;
Could do "else if" here. Also, why not "minimized" and "maximized"? (In which
case, I'd recommend using NS_LITERAL_STRING's...)
Reporter | ||
Comment 24•24 years ago
|
||
else-if and NS_LITERAL_STRING. Check.
I used "m" and "M" to match those values that are already being persisted
elsewhere. I could change the persistence code, too, if you like.
Comment 25•24 years ago
|
||
This bug was triaged rtm- on 10/4. It remains rtm-.
Whiteboard: [rtm need info] patch attached → [rtm-] patch attached
Target Milestone: M19 → Future
Comment 26•24 years ago
|
||
I don't like the values "m" and "M". I think we should use "minimized" and
"maximized" as the values, even if it involves changing more code. This has no
hope of being rtm+, so we may as well do it right.
Reporter | ||
Comment 27•24 years ago
|
||
Trust Peter to push process for process' sake. Restoring the helpwanted flag.
External Mozilla developers, you know what to do.
Keywords: helpwanted
Comment 28•24 years ago
|
||
No, this is minimal process for the sake of shipping product. As far as I could
tell, the triage on this bug was changed for unknown reasons, perhaps even
accidentally. We need to keep such triage visible and rational, and the
decision has to include the assigned engineer, jrgm and myself, plus ekrock if
he has expressed interest. If these people think this bug is severe enough, and
the patch safe enough, to pass PDT criteria for N6, then I'm willing to let that
happen. Even then, I think the concerned development and QA engineers will need
to present their case directly to PDT, and the result is still almost certain to
be 'rtm-'.
Comment 29•24 years ago
|
||
At the risk of upsetting everyone i'm resetting the severity.
hyatt/waterson:
Instead of maximized and minimized which are really nice in an English speaking
world [and potentially painful to anyone who has w/ English], ...
Could we use X and N [or n]?
maXimized, miNimized.
brendan: helpwanted, this patch sounds good and appears to have a good
architecture [otherwise why would people complain about the state variable
values], but we don't even have it on the trunk :-( ... Could you help work out
this last kink?
danm: thank you very much for the patch, i'm sorry that it wasn't perfect
enough for some ... let's see if we can get this into the trunk.
Blake: important patches that have minimal work left, sound interesting?
Severity: minor → normal
Comment 30•24 years ago
|
||
Comment 31•24 years ago
|
||
I'm requesting review of my patch.
If I get it checked in I will change this to [rtm+] since i have just stolen
the bug from danm per danm@netscape.com 2000-10-18 09:58.
I'm setting a milestone too.
As this is my bug, a few ground rules:
(a) if you have questions contact me on irc
(b) don't apply netscape management rules to my bugs
(b1) pdt [rtm++] still applies
(b2) pdt should not consider this bug until I say it is ready
(b3) don't mess with the [rtm*] in the status while it is my bug
(c) please don't spam my bug(s), that's what newsgroups and irc are for.
Please excuse my rules, but other related bugs got into big fights over this
nonsense, so i'm putting these here to prevent that.
wrt #define I think that we could easily change that after this bug was checked
into trunk, IMO testing and usability is more important on trunk than a bit of
style in a header file. I talked to people on irc and #define seemed vaguely
acceptable I think a const nsLiteralString might be prefered but i'm not sure.
scc: comments?
Assignee: danm → timeless
Status: ASSIGNED → NEW
Target Milestone: Future → mozilla0.6
Comment 32•24 years ago
|
||
Repeating waterson's review, better pay attention:
+
+
if(NS_SUCCEEDED(docShellElement->GetAttribute(NS_ConvertASCIItoUCS2("sizemode"),
sizeString)))
Don't use NS_ConvertASCIItoUCS2, it wastes malloc space and cycles on platforms
that have two-byte L"foo" characters. Use NS_LITERAL_STRING.
+ {
+ mSizeMode = nsSizeMode_Normal;
+ if (sizeString.EqualsWithConversion(wsmMinimized))
Use Equals, not EqualsWithConvertion, as there is no conversion required now
that you are using NS_LITERAL_STRING (inside the wsmMinimized macro).
Name your constants using ALL_CAPS, not interCaps: WSM_MINIMIZED, etc.
+ mSizeMode = nsSizeMode_Minimized;
+ if (sizeString.EqualsWithConversion(wsmMaximized))
As waterson pointed out, there should be an else before this if.
+ mSizeMode = nsSizeMode_Maximized;
+ // defer telling the widget until we're visible
+ }
Same comments apply here:
- sizeString.AssignWithConversion("n");
+ sizeString.AssignWithConversion(wsmNormal);
if (sizemode == nsSizeMode_Minimized)
- sizeString.AssignWithConversion("m");
+ sizeString.AssignWithConversion(wsmMinimized);
else if (sizemode == nsSizeMode_Maximized)
- sizeString.AssignWithConversion("M");
+ sizeString.AssignWithConversion(wsmMaximized);
About the following, in addition to renaming the macros to use Java/JS/Mozilla
house style (ALL_CAPS), I agree with hyatt: why are we using the cybercrud
school of abbreviation? Why not "minimized", "normal", and "maximized"?
+#define wsmMaximized NS_LITERAL_STRING("X");
+#define wsmNormal NS_LITERAL_STRING("n");
+#define wsmMinimized NS_LITERAL_STRING("m");
/be
Comment 33•24 years ago
|
||
danm, i'm starting to really sympathize
brendan: thanks, BUT danm's patch was 'in the style of' the file he was
patching. I'll attach my new patch, but here's my list of things that i didn't
patch (I request that you read the entire file and give comments here on any
other things to which you object, I'll fix them too if given the proper
guidance)
many instances of this:
PR_snprintf(sizeBuf, sizeof(sizeBuf), "%ld", (long)x);
sizeString.AssignWithConversion(sizeBuf);
NS_ENSURE_SUCCESS(domdoc->GetElementById(NS_ConvertASCIItoUCS2(aID),
aDOMElement), NS_ERROR_FAILURE);
static const char *prefix = "@mozilla.org/appshell/component/browser/window;1";
nsAutoString topic; topic.AssignWithConversion(prefix);
topic.AppendWithConversion(";");
nsCOMPtr<nsIJSContextStack> stack(do_GetService
("@mozilla.org/js/xpc/ContextStack;1"));
nsCOMPtr<nsIScreenManager> screenmgr = do_GetService
("@mozilla.org/gfx/screenmanager;1", &result);
prefres = prefs->CopyCharPref("browser.chromeURL", &urlStr);
urlStr = "chrome://navigator/content/navigator.xul";
nsCOMPtr<nsIJSContextStack> stack(do_GetService
("@mozilla.org/js/xpc/ContextStack;1"));
NS_ENSURE_SUCCESS(service->Notify(removeme, topic.GetUnicode(), aData),
NS_ERROR_FAILURE);
scc: if you find me on irc and have time to tutor me, i'd appreciate it.
Patch3 incoming.
Status: NEW → ASSIGNED
Comment 34•24 years ago
|
||
Comment 35•24 years ago
|
||
danm, asside from #define waterson thinks i've fulfilled his requests
please review (r=). If you have a prefered style for the constants I'll use it,
I was thinking that some sort of constant static window class element would be
more correct but I don't know the style for that.
Comment 36•24 years ago
|
||
timeless: I don't care whose patch was in what style, I comment on bad or just
old practice and style, and the presence of it elsewhere in the file is no
excuse for doing the wrong thing when adding or modifying. See rule/tip #13 in
http://www.mozilla.org/hacking/reviewers.html#rules-and-tips, especially the
link to new string tech.
It's a good idea to look at the rest of the file, too -- I was lazy, and didn't
want to turn over a rock in case this bug had any chance of getting [rtm+]'d.
Right now I'm still short on time to review the whole thing, but some quick
comments on your comment:
PR_snprintf(sizeBuf, sizeof(sizeBuf), "%ld", (long)x);
sizeString.AssignWithConversion(sizeBuf);
There is indeed a conversion from ASCII to pseudo-UCS2, but scc has a better way
to do this now: nsPrintfCString. I hope it can be used here without trouble; I
will let scc take over on this one.
NS_ENSURE_SUCCESS(domdoc->GetElementById(NS_ConvertASCIItoUCS2(aID),
aDOMElement), NS_ERROR_FAILURE);
Looks like a bone fide conversion, not a string literal. Or did you want me to
barf on the NS_ENSURE_SUCCESS with its hidden return statement?
static const char *prefix = "@mozilla.org/appshell/component/browser/window;1";
nsAutoString topic; topic.AssignWithConversion(prefix);
topic.AppendWithConversion(";");
I think we need another scc jihad through the tree to avoid gratuitous
conversions from string literals that may be expressible in 2-byte Unichars as
literals.
nsCOMPtr<nsIJSContextStack> stack(do_GetService
("@mozilla.org/js/xpc/ContextStack;1"));
nsCOMPtr<nsIScreenManager> screenmgr = do_GetService
("@mozilla.org/gfx/screenmanager;1", &result);
prefres = prefs->CopyCharPref("browser.chromeURL", &urlStr);
urlStr = "chrome://navigator/content/navigator.xul";
nsCOMPtr<nsIJSContextStack> stack(do_GetService
("@mozilla.org/js/xpc/ContextStack;1"));
NS_ENSURE_SUCCESS(service->Notify(removeme, topic.GetUnicode(), aData),
NS_ERROR_FAILURE);
Not sure what to say about these. Some look just fine.
/be
Comment 37•24 years ago
|
||
I can't believe travis polluted every file he touched with his fibonacci number
indentation and bracing.
Latest patch looks good to me, apart from that mess.
/be
Comment 38•24 years ago
|
||
I want "minimized", "normal", and "maximized" as the values. This is consistent
with other attribute values in XUL (we use full English words for all XUL
attribute values and not abbreviations).
Reporter | ||
Comment 39•24 years ago
|
||
The logic of timeless' patch looks oddly familiar, and fine by me. Reviewers
have been particular about the details, though. So as long as we're still ripping
on you, I'll express a small desire to see the constant's you're defining moved
out of the header file into the source. By my way of thinking, they're more an
internal implementation issue; not really part of the interface.
I'll also echo Brendan's indentation comment: as long as you're touching nearly
every line of LoadPositionAndSizeFromXUL, please do restore it to its pre-Travis,
Mozilla-standard 2 space indentation, and your choice of any brace style in
common usage.
My reading of the comments is that the latest patch will meet everyone's
expectations if timeless uses nsPrintfCString, reformats the much-touched
function to meet Mozilla standards, considers moving the #defines to the source
file, and heeds David about the full-on English attribute values. (Yes, that
makes them less international, but there's no getting around XUL files' highly
English bent.)
As far as I'm concerned, there's no need to prove your intentions by posting
yet another patch. r=danm.
Hey, hang in there. You're almost done.
Comment 40•24 years ago
|
||
Comment 41•24 years ago
|
||
Ok, I made one coding error
s/(#define.*);/$1/ to undo
After this fix the code compiles and apparently works for Jerry_Baker so all i
need is scc's explanation of string handling
Hyatt: your bone was fixed before you repeated your comment. I'm sorry you
didn't check my latest avail patch
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=17530.
Whiteboard: [rtm-] patch attached → [rtm-] (error in patch attached) Waiting for SCC comments
Comment 42•24 years ago
|
||
+ default :
+ mode = SW_RESTORE;
+ }
Please put a break; after the default: case, even though it is last (compiles
with half a brain will eliminate the branch-to-next-instruction). Recall the
tale from the '80s (comp.risks archives) where a giant AT&T central office
switch crashed because of a missing break after default (and then someone added
a case after the default).
What is going on here with control flow?
+ if (aIID.Equals(NS_GET_IID(nsIPrompt))) {
+ // XXX until nsIWebShellWindow goes away:
+ nsCOMPtr<nsIWebShellWindow> webShellWin =
+ do_QueryInterface(NS_STATIC_CAST(nsIXULWindow*, this), &rv);
+ if (NS_FAILED(rv)) return rv;
+ return webShellWin->GetPrompter((nsIPrompt**)aSink);
+ }
+ if (aIID.Equals(NS_GET_IID(nsIWebBrowserChrome)) &&
+ NS_SUCCEEDED(EnsureContentTreeOwner()) &&
+ NS_SUCCEEDED(mContentTreeOwner->QueryInterface(aIID, aSink)))
+ return NS_OK;
+ else
No braces around multi-line if/then, and else after return is a non-sequitur,
but worse: if aIID is nsIWebBrowserChrome, but any of the && clauses after that
test fail, you fall into code that tries to compare aIID to other IIDs. I think
you (we, really -- you didn't write this, but we're revising it ;-) want:
+ if (aIID.Equals(NS_GET_IID(nsIWebBrowserChrome))) {
+ rv = EnsureContentTreeOwner();
+ if (NS_FAILED(rv)) return rv;
+ return mContentTreeOwner->QueryInterface(aIID, aSink);
+ }
Now recall that we left off with an else after return non-sequitur:
+ else
+ return QueryInterface(aIID, aSink);
- NS_IF_ADDREF(((nsISupports*)*aSink));
- return NS_OK;
+ NS_IF_ADDREF(((nsISupports*)*aSink));
+ return NS_OK;
The final code, the NS_IF_ADDREF and return NS_OK, is not reachable! Either do
just the following in place of the above:
+ return QueryInterface(aIID, aSink);
or tell me what I'm missing here.
+NS_IMETHODIMP nsXULWindow::GetDocShell(nsIDocShell** aDocShell) {
+ NS_ENSURE_ARG_POINTER(aDocShell);
Could we have the opening brace of all method bodies on a newline, by itself,
and unindented? This special case for an opening brace (which you, I, and much
of this code typically puts on the same line as the if, for, while, switch, or
do keyword introducing the block) allows readers and grep-like automation (vim
[[ in particular) to find the opening brace of a function body fast. Same for:
+NS_IMETHODIMP nsXULWindow::GetZlevel(PRUint32 *outLevel) {
+ *outLevel = mZlevel;
+ return NS_OK;
}
Looks like you changed more methods after this one, too. Oops, you did the same
thing to GetInterface's opening brace, but I didn't notice above. Really, the
special case where a method or function opening brace goes on its own line
should be preserved in this file, I think.
More in a bit, have to get my car.
/be
Comment 43•24 years ago
|
||
Timeless, thanks again for taking on this cleanup. Back to the code:
+ nsContentShellInfo* shellInfo =
(nsContentShellInfo*)mContentShells.ElementAt(i);
NS_STATIC_CAST(nsContentShellInfo*, mContentShells.ElementAt(i)) instead of
old C-style cast here.
+ if (shellInfo->primary)
{
- nsContentShellInfo* shellInfo =
(nsContentShellInfo*)mContentShells.ElementAt(i);
- if(shellInfo->primary)
- {
- *aDocShellTreeItem = shellInfo->child;
- NS_ADDREF(*aDocShellTreeItem);
- return NS_OK;
- }
+ *aDocShellTreeItem = shellInfo->child;
+ NS_ADDREF(*aDocShellTreeItem);
+ return NS_OK;
}
+ }
Bracing looks funky (travis-style, indented to same level as the block's code,
and each brace on its own line).
Next method:
+ PRInt32 count = mContentShells.Count();
+ for(PRInt32 i = 0; i < count; i++) {
+ nsContentShellInfo* shellInfo =
(nsContentShellInfo*)mContentShells.ElementAt(i);
+ if (shellInfo->id.Equals(aID)) {
+ *aDocShellTreeItem = shellInfo->child;
+ NS_ADDREF(*aDocShellTreeItem);
+ return NS_OK;
}
+ }
Closing braces don't line up with opening control keywords. Same prob later, in
ShowModal. At this point, how about fixing what I've commented on, and then
attaching diff -u and diff -wu patches (the latter will help see past much of
the tab/indentation fixing). Thanks,
/be
Comment 44•24 years ago
|
||
Comment 45•24 years ago
|
||
Comment 46•24 years ago
|
||
Brendan, not that anyone cares about the mn6 branch, but do you think i'd
actually be able to check in my complete rewrite of these files on that branch?
I think i'd like to be able to check in a basic version of this patch (once i
get instructions on using that function from scc, or maybe w/o that) to the mn6
branch..
Comment 47•24 years ago
|
||
Umm ... Just to clarify, this patch *just* persists maximized state (as in the
original report), not minimized state, right? Persisting minimized state would
be very confusing for the user.
Comment 48•24 years ago
|
||
adding Self to CC. Sorry for the spam.
Comment 49•24 years ago
|
||
why nobody's checking this in? at least in the trunk! would like to vote, but
don't have votes any more.
Comment 50•24 years ago
|
||
timeless: there's a way-too-long line after:
@@ -409,48 +407,13 @@
// nsWindow constructor
//
//-------------------------------------------------------------------------
-nsWindow::nsWindow() : nsBaseWidget()
The member initializers should be broken up into 80-char line runs, or even
better, one member per line.
This hanging indented arguments example and the one after it need to underhang
the first argument (indented to the column after the opening parenthesis):
@@ -1151,8 +1106,7 @@
nsWidgetInitData *aInitData)
{
return(StandardWindowCreate(aParent, aRect, aHandleEventFunction,
- aContext, aAppShell, aToolkit, aInitData,
- nsnull));
+ aContext, aAppShell, aToolkit, aInitData, nsnull));
}
I think the - lines are closer to ideal, and just need to be indented a bit more
to the right. Another example, from the diff -u patch:
+ if (NULL != deferrer) {
+ VERIFY(((nsWindow *)par)->mDeferredPositioner =
::DeferWindowPos(deferrer,
+ mWnd, NULL, aX, aY, 0, 0,
+ SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOSIZE));
+ } else {
+ VERIFY(::SetWindowPos(mWnd, NULL, aX, aY, 0, 0,
+ SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOSIZE));
}
The assignment expression in the "then" part has a long left-hand side, so it
seems best to break after the = operator:
+ if (NULL != deferrer) {
+ VERIFY(((nsWindow *)par)->mDeferredPositioner =
+ ::DeferWindowPos(deferrer, mWnd, NULL, aX, aY, 0, 0,
+ SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOSIZE));
+ } else {
+ VERIFY(::SetWindowPos(mWnd, NULL, aX, aY, 0, 0,
+ SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOSIZE));
}
and indent the right-hand side one more unit (two spaces). I also fixed the
hanging indent in the "else" clause's multiline actual argument list.
Total nit below, but I think most people do not put a space before the colon
that ends a case label (or a goto label):
+ case nsSizeMode_Maximized :
+ mode = SW_MAXIMIZE;
+ break;
Certainly, other switches in this file do not have such spaces.
Here's a case where the long argument expression should be on the second line of
arguments, underhanging the first arg, and not split across the line break:
@@ -2121,8 +2098,8 @@
NS_IMETHODIMP nsWindow::ScrollWidgets(PRInt32 aDx, PRInt32 aDy)
{
// Scroll the entire contents of the window + change the offset of any
child windows
- ::ScrollWindowEx(mWnd, aDx, aDy, NULL, NULL, NULL,
- NULL, SW_INVALIDATE | SW_SCROLLCHILDREN);
+ ::ScrollWindowEx(mWnd, aDx, aDy, NULL, NULL, NULL, NULL, SW_INVALIDATE |
+ SW_SCROLLCHILDREN);
Just put the SW_INVALIDATE | SW_SCROLLCHILDREN on the second line.
Thanks for undertaking this cleanup, it's big and daunting. I suggest more
divide and conquer (one method at at time, or only affected methods) next time.
Eventually, the whole file will be much more readable, but you'll have an easier
time getting reviews.
I'll review a new patch and get blizzard to sr= it.
Please assuage mpt's concern about minimized state persistence.
/be
Comment 51•24 years ago
|
||
As a Mozilla Advocate, I have to say that one of the most annoying things about
using mozilla for the past year is this bug. PLEASE, if at all possible get an
RTM++ for this so it is fixed in Netscape 6!
Please!
Comment 52•24 years ago
|
||
Comment 53•24 years ago
|
||
A few people broke the rules set forth in this bug by me in Comments From
timeless@mac.com 2000-10-18 17:20
As this is my bug, a few ground rules:
(c) please don't spam my bug(s), that's what newsgroups and irc are for.
Thank you for wasting my time, and upsetting me. You were removed. If you
quietely add yourself to the cc list I will not remove you. However, I would
suggest you use silent voting instead, because in the future the CC list might
be used by the developer (I am the developer for this bug as of that date).
Attached is a zip containing a cvs tip diff in -u and -wu format.
Brendan said he would review.
scc: please check the diff -wu for any conventions that you would like fixed.
someone: please test this patch.
everyone else: I will have a more complete set of bug commenting guidelines
which you should read before commenting on any bugs, but especially mine.
mpt: If you can explain to me how someone could send netscape a newwindow
signal that would cause this code to load a window minimized then you may
explain it in a future bug. This patch has already become way too hard to
manage which is why it took so long to work on.
I suspect that any such path is a bug in something else, and therefore I will
not fix it now, nor will I fix it in response to this bug which clearly says
to preserve both states.
i'm adding a rule (d).
(d) don't change the scope or purpose of my bugs.
This means that after this fix is committed (if it ever gets committed), anyone
who finds a flaw in it may feel free to assign a new bug to me as caused by
this bug (so this bug would block your new bug). This is standard procedure,
but I feel the need to spell it out as other people have broken similarly
obvious guidelines.
Keywords: helpwanted,
mozilla0.9,
rtm
Whiteboard: [rtm-] (error in patch attached) Waiting for SCC comments
Target Milestone: mozilla0.6 → mozilla0.9
Comment 54•24 years ago
|
||
This patch should not be allowed to land if it persists the minimized state.
That would be a far worse regression than the defect it is intended to fix.
Comment 55•24 years ago
|
||
*** Bug 60057 has been marked as a duplicate of this bug. ***
Comment 56•24 years ago
|
||
why shouldn't it persist minimized state? Ok, a few novices in windows/mac use
won't see that there's mozilla in the task menu, but this would be a much less
problem than the current one and the bug summary says it _should_ persist the
current state.
And the only case it is possible to persist minimized state is startup because
you can't open a new window out of a minimized window or am I wrong?
Comment 57•24 years ago
|
||
Windows shouldn't persist minimized state because:
(1) If my current Mozilla window is minimized and I choose `File' > `New' >
`Navigator Window', or press Ctrl+N, if the new window is minimized I'll be
quite annoyed. (4.x behavior is, correctly, to open a non-minimized window in
this case.)
(2) If Mozilla windows are minimized when grandma shuts down Windows, and then
when she next runs Mozilla it persists that state and opens its windows
minimized, she'll be hopelessly confused.
Comment 58•24 years ago
|
||
> If my current Mozilla window is minimized and I choose `File' > `New'
> `Navigator Window', or press Ctrl+N,
How is this possible if the window is minimised?
I see your point about 2), though.
Gerv
Comment 59•24 years ago
|
||
2. is outside the scope of the bug because i don't expect this persistance to
happen beyond the current session.
If i am wrong i am sure danm or I can fix it so that when moz starts it ignores
minimization.
Comment 60•24 years ago
|
||
Changing summary; it would be acceptable to keep track of whether the last state
was minimized, and some niche-market utilities may actually want to persist that
for some strange reason, but this bug was originally filed for persisting only
the maximized/normal states, and that is what the default behavior should be for
the current suite of apps. Doing otherwise would hopelessly confuse 99.44% of
all users (including mozillians), not just the mythical 'grandma'.
Summary: window maximized/minimized state not persisted → window maximized state not persisted
Comment 62•24 years ago
|
||
It looks to me like this bug was originally filed with the summary "window
maximized/minimized state not persisted". timeless (the bug's assignee) put in
quite a bit of effort pushing the process forward and making this happen. In
his 2000-11-13 15:42 comments the assignee specifically requested that other's
"don't change the scope or purpose of my bugs." My read of the process is that
the bug's summary belongs initially to the reporter, (possibly briefly to QA and
Development as the bug is triaged) and comes to rest sqarely with the developer
the bug is assigned to. Changing the summary and scope of a bug without the
assignee's permission (especially after patches are beign reviewed) is
inappropriate and dangerous.
I fail to see where "this bug was originally filed for persisting only
the maximized/normal states". The only summary change I can find in teh Bug
History is:
trudelle@netscape.com changed | short_desc window maximized/minimized state not
persisted | window maximized state not persisted | 2000-12-04 17:32:03
I'm deeply concerned that a contributor to this project who had been working
diligently within the review and approval process on a bug which is as high
profile as this was shown so little respect.
-Asa
Comment 63•24 years ago
|
||
per timeless's good reporting rules, just cc:ing myself!
Comment 64•24 years ago
|
||
I don't understand what the issue is here. Ensuring that a new patch does not
cause another problem before checking it in doesn't seem like an unusual
request to me. It doesn't seem like a good idea to rush to check this in and
then worry about the problem it will cause later on. I understand that the
original summary said to persist minimized state, but dan didn't even mention
it in the description, and the consensus now seems to be that that would be
harmful. Let's just fix the patch to keep track of, but not actually restore,
the state if it's minimized...
Comment 65•24 years ago
|
||
There is certainly not a consensus about the harms of minimization. No one has
given a valid reason for not persisting minimized state. The only way to find
out if it is bad is to try it. If somoene builds the binaries (i'll try
building this weekend, I just got MSVS last week and I have projects and papers
to write first).
Comment 66•24 years ago
|
||
I have seen valid reasons. I would think the fact that no other app does it,
along with the fact that it can be extremely confusing, would be reason
enough. Yes, the patch needs to be tried. If there isn't a problem, we're
arguing over nothing. If there is, it should be fixed.
Comment 67•24 years ago
|
||
The original text of the bug report mentions only maximized and normal state. I
think it is inappropriate for a 'summary' to include something not in the full
report, especially something that could cause a worse defect than the one
reported. As the module owner, I think I have some ownership of this bug as
well, but I'd be glad to abide by the accepted mozilla.org processes and
ownership models for changing bug fields, that I have supposedly disregarded, if
someone would kindly point out where they are defined. I certainly meant no
disrespect to Timeless, and do appreciate his work on this patch while DanM was
prevented from checkin it in.
reassigning back to the DanM, who should still find the attachment 'oddly
familiar', although nicely scrubbed.
Assignee: trudelle → danm
Comment 68•24 years ago
|
||
*** Bug 62329 has been marked as a duplicate of this bug. ***
Comment 69•24 years ago
|
||
Comment 70•24 years ago
|
||
<sulk>
mumblegrumbleMacgrumblemumble.
<sigh> OK, so the ideal solution is not to persist minimised. However, I still
think we should check this patch in and then tweak it later :-)
Gerv
Comment 71•24 years ago
|
||
Is this bug related to the fact that you can get zero size windows by clicking
on an url in your favourite mail program while Mozilla is minimized? (Well,
they aren't completely zero size. Windows won't let them go any smaller than
what's required to display the window's title bar).
Comment 72•24 years ago
|
||
I hope not, since this patch was never checked in.
German/mpt. interesting point. I don't think this is enough to prevent
persistance of state, i think it means that the specific chrome based entry
points should tell the new window mechanism not to persist minimized.
Cases so far:
[don't consider minimized]
Chrome
commandline/dde
[do consider minimized]
webjavascript
Are there any other cases?
Reporter | ||
Comment 73•24 years ago
|
||
It's a joy to have this bug back.
Take fifteen. Here's a patch much like the original, simpler patch from 17 Oct
except it won't minimize a window and it addresses Chris Waterson's objections
to that patch. Oh, and I was mistaken in earlier claims where I said nothing
need be done on the Mac. This one covers the Mac, too. Nothing happens on Linux
because there's no specific zoom state, as far as I know.
I made no attempt to un-Travis the indentation or braces. Forgive me; I
even stuck with the Travis indentation where new code was surrounded by
them. Don't really want to reformat the file right now. Lookin' for reviews once
more.
Status: NEW → ASSIGNED
Whiteboard: patch awaiting review
Reporter | ||
Comment 74•24 years ago
|
||
Reporter | ||
Comment 75•24 years ago
|
||
Oh, and there's something wrong with RDF persistence of window state right now.
That prevents this patch from working quite right. I'll get around to tracking
that down someday, too. Sigh.
Comment 77•24 years ago
|
||
I didn't install the patch, but the Windows code looks like it ought to do the
trick.
Comment 78•24 years ago
|
||
r=hyatt
Comment 79•24 years ago
|
||
mac part looks good, r=pink
Comment 80•24 years ago
|
||
sr=brendan@mozilla.org.
I'd still like to get the main good effects of timeless's patch in. I'll help.
Dan, you up for it?
/be
Reporter | ||
Comment 81•24 years ago
|
||
yes. a different checkin for the cleanup would be cleaner. i'm leaving the bug
open until we get that part fixed, too.
Comment 82•24 years ago
|
||
Nothing has changed in terms of window state persistence in my latest win32 tip
build (or in a nightly I just downloaded). Is this because of the RDF
persistence bug you mentioned, or...?
Reporter | ||
Comment 83•24 years ago
|
||
Man, it's sure working for me now. Your sidebar must be open to see it; bug
61471 is preventing flushing of window state on application close if the sidebar
is closed.
That said, my windows build this morning is showing an embarrassing problem
with redrawing the window contents after maximizing. Sigh. I swear it wasn't
there yesterday.
Whiteboard: patch awaiting review
Comment 84•24 years ago
|
||
*** Bug 62984 has been marked as a duplicate of this bug. ***
Comment 85•24 years ago
|
||
I just downloaded build 2000121508 because this is a fix I've been waiting on
for a while... I loaded it, maximized the window, enabled the sidebar, and
exited. Then I loaded and disabled the sidebar. All new browser windows are
opening maximized.
Unfortunately, minimizing a maximized browser window and then restoring it
causes it to lose its maximization. Was this regression caused by the fix to
this bug?
Comment 86•24 years ago
|
||
Still doesn't work for me in win tip build or a new installer nightly I just
downloaded. Deleted profile, etc. Opened sidebar. Do I suck or what?
Comment 87•24 years ago
|
||
Uh oh. I see what's happening. This works for me if I use the Windows
restore/maximize buttons (upper right corner) to change the state, but not if I
double click the titlebar, which is my usual method of changing window state.
Looking at the patch, I don't understand why that would be...do we not get
notification in that case? I'll split off a separate bug for this.
Comment 88•24 years ago
|
||
This works *sometimes* for me.
It seems that the state of the last closed window is persisted, not the state
of the current window.
Try this:
1. Launch Mozilla.
2. Open a Nav window, maximize it.
3. Open a new window via Ctrl-N (or right click on a link and open it in a
new window)
4. Make sure that the new window opened is not maximized.
5. Close that window.
6. Go back to your original maximized window.
7. Open a new window again (Ctrl-N).
Expected result: New window is opened in a maximized state.
Actual result: New window is opened in the restored state (the state in which
the window closed in step 5 was).
This is a 4.x parity concern. In N4, pressing Ctrl-N or otherwise opening a new
window from a maximized window, resulted in the new window being maximized, no
matter the state of the last window closed.
In other words, although it is great progress that the maximized state of a
last closed window is persisted, it is, however, necessary to perist the state
of the *current* window, not the last window, during "new window" commands
(i.e. Ctrl-N via keyboard or menu, right-click and select Open In New Window,
clicking on a link with window="_top").
Comment 89•24 years ago
|
||
> Expected result: New window is opened in a maximized state.
> Actual result: New window is opened in the restored state (the state in which
> the window closed in step 5 was).
This is arguable.
> This is a 4.x parity concern. In N4, pressing Ctrl-N or otherwise opening a
> new window from a maximized window, resulted in the new window being
> maximized, no matter the state of the last window closed.
No, on Linux, it behaves just as you describe the current "Actual result" of
Mozilla. I guess, you are speaking of Windows. So, you cannot speak about 4.x
parity.
Comment 90•24 years ago
|
||
I think that 4.x got this behavior very wrong, I always found it maddening that
it would continue to open windows the size of the last window closed, even if I
resized each new one. It persisted the window state only on closing, I believe
due to programmer convenience. In order to get new windows to open as they want,
users would have to carefully size one, then *close it*?!! That seems very
counter-intuitive, and a huge waste of users' time. It should instead try to
persist the most recent sizing for the same type of window. Perhaps this needs
to be a separate bug? I think I've filed it before...
Comment 91•24 years ago
|
||
So, what should the code do, exactly? Persist the size of the last manually
resized window?
Comment 92•24 years ago
|
||
I think what most people expect it to do is to copy the size/state of the
current window. For example, if I'm in a maximized window, and I hit ctrl-n, I
want another maximized window. However, if I'm in a small window, I'd like
another small window when I open a new window.
Comment 93•24 years ago
|
||
That sounds reasonable if you're opening the same type of window as the current
window. (i.e., Ctrl-N in mail should not open a browser window the size of the
current mail window.)
Comment 94•24 years ago
|
||
That's not true, if the user didn't set the window size himself. If some
ill-minded web application programmer opened a window with a certain size for
me, and I say "new window", I want a normally sized window.
Note that your suggestions is no persistantce at all technically, at least not
during runtime. The new window just inherits the size of the old one.
Comment 95•24 years ago
|
||
I suggest if you have multiple windows of one kind of different sizes in the
session you should not try to persist those sizes, and you should just leave the
last saved value as it is.
But I agree that you should open a window at the same size if it's the same
kind, or all other windows of that kind are of one size.
No longer depends on: 63268
Comment 96•24 years ago
|
||
Looks like I just clobbered danm's dep change. Are dependency changes not
considered in midair collisions?
Restoring.
Depends on: 63268
Comment 97•24 years ago
|
||
What about only persisting size for windows that the user personally resized.
Ie if a javascript popup occurs, it is not counted as a user resize.
Comment 98•24 years ago
|
||
But when would the state get saved and when will it be used? Suppose I have one
maximized window, then open up another window, resize it, and go back to my
original, maximized window and hit ctrl-n. If this does anything else than give
me a new maximized window, I'll be highly annoyed.
Comment 99•24 years ago
|
||
From the patch, in windows/nsWindow.cpp:
+ switch (aMode) {
+ case nsSizeMode_Maximized :
+ mode = SW_MAXIMIZE;
+ break;
+ case nsSizeMode_Minimized :
+ mode = SW_MINIMIZE;
+ break;
+ default :
+ mode = SW_RESTORE;
Should that last one be SW_NORMAL instead? Probably makes no difference in
practice, but just a thought.
Reporter | ||
Comment 100•24 years ago
|
||
Dean: SW_SHOWNORMAL, right? I believe the distinction boils down to whether the
window is already visible. And all three cases similarly have dual constants.
They could arguably all three go something like
case nsSizeMode_Maximized :
mode = mIsVisible ? SW_MAXIMIZE : SW_SHOWMAXIMIZED;
However in practice I haven't noticed that any of this makes any difference.
Still, see the patch in bug 63268, which addresses this while it fixes another
related problem.
All: recent discussion in this bug seems to have turned to the question of
which window has its size and state persisted. At the moment, it's the last
window that changed, which isn't quite right. Making it right should be as simple
as persisting the state info when a window is activated. This should be a one-
line change, but the unpleasant nsWebShellWindow/nsXULWindow dichotomy makes this
a little more difficult.
I can get around to fixing this soon but not immediately. If someone beats me
with a patch, that'd be OK, too.
Comment 101•24 years ago
|
||
Dan, it actually makes no difference.
SW_NORMAL == SW_SHOWNORMAL == 1
SW_MAXIMIZE == SW_SHOWMAXIMIZED == 3
It never occurred to me about mIsVisible. I think that using ShowWindow will
always show a hidden window, unless you specify SW_HIDE.
Maybe if mIsVisible == PR_FALSE then don't call ShowWindow, but when mIsVisible
changes, call ShowWindow with the new mode.
Comment 102•24 years ago
|
||
Hrm, is it just me or does this only work for browser windows, and only when you
open them manually (as opposed to obnoxious web sites which use "target" to open
windows)?
Reporter | ||
Comment 103•24 years ago
|
||
Not sure what you mean by obnoxious websites' windows. We explicitly do not
persist size info for spam windows, to keep them from changing your browser
settings. Saving special settings for spam windows is a thought -- what, minimize
one and have them collect in the taskbar from then on, instead of opening? But
that would be a Request For Enhancement.
As for only affecting the browser window, yes, that's true. The feature is
enabled, and can now easily be turned on for your favourite window. It should
probably be turned on for all the Task windows.
Comment 104•24 years ago
|
||
I was referring to sites that use target=_blank (or similar) to open new windows
when you click on a link. They don't get maximized, which makes a bad site worse
:) 4.x maximizes them, but I don't recall what MSIE does.
Comment 105•24 years ago
|
||
A new window explicitly created from an existing window (i.e. by pressing
Ctrl+N, or choosing `Open Link in New Window') should inherit all the
properties of the existing window, including maximized/restored state (but not
minimized state). (Note that `explicitly created' does not include windows
which open by themselves, e.g. JS popups or target="_new". Internet Explorer
always opens these in a non-maximized window.)
A new window created from nowhere (e.g. by double-clicking the Navigator icon,
or by pressing Ctrl+N when no windows are open) should inherit all the
properties of the window where the state was last manually changed by the user.
Comment 106•24 years ago
|
||
"Note that `explicitly created' does not include windows
which open by themselves, e.g. JS popups or target="_new". Internet Explorer
always opens these in a non-maximized window."
IE always opens target="_blank" or "_new" windows unmaximized, as it also does
to windows opened through "Open Link in New Window". I always thought these were
among IE's most annoying "features".
All new windows should assume the user wants them to be the same size as the
last window, except those explicitly declared to be different through Javascript.
Comment 107•24 years ago
|
||
For what reason would anyone ever want new windows to open maximized, yet not
want windows produced by target=_blank to open maximized? I'm not clear on what
circumstances would make this behavior appropriate.
Reporter | ||
Comment 108•24 years ago
|
||
I love this bug. Glad to see there are so many different opinions. Here's what
you're going to get from me before I close it "fixed." It follows the opinions of
some. I mean besides just me. Ah, the warm glow.
New windows will be opened to the specifications of the most recently active
window of the same type (browser, mail, ...). Except for the Linux build, which
(Pavlov tells me) doesn't provide any decent, cross-window-manager way of
informing the app that a window has been maximized or un-, and prefers to allow
the window manager to position the new window. Linux will inherit window size,
only.
That's it. It's arguably not quite right. It's arguably totally right. People
are doing both. Those on my side can share that warm glow I'm working on.
Comment 109•24 years ago
|
||
> All new windows should assume the user wants them to be the same size as the
> last window, except those explicitly declared to be different through
> Javascript.
I don't know what you mean with "explicitly", but we must not assume the user
wants something, unless he explicitly (i.e. manually, through the Mozilla
chrome) triggered it himself. Again: If my bank account access app opened a
window of size 400x300, why would I want to have subsequent windows open in the
same size??
> If you For what reason would anyone ever want new windows to open maximized,
> yet not want windows produced by target=_blank to open maximized?
Because I newbie might not notice at all, that a new window opened.
> I love this bug.
I can very well imagine that. :-(
> New windows will be opened to the specifications of the most recently active
> window of the same type
> Except for the Linux build, which [...] will inherit window size, only.
Windows don't inherit the screen *position* (x,y coordinates of the top leflt
corner), do they? This would be a serious usability bug, because the user might
not notice at all, that a new window opened. (I know, *I* was sometimes
confused, even.) But IIRC, this problem preexists your fix. Is there another bug
about it?
Comment 110•24 years ago
|
||
To relief danm's nerves and follow german's reasonable suggestion: Please post
replies to .ui (I posted my comment there
<news://news.mozilla.org/3A441714.30609@bucksch.org>).
Comment 111•24 years ago
|
||
Should not this bug include not only browser windows, but also Mail, Composer
and Address Book windows?
I notice that in the latest builds, the browser window remembers its maximized
state, but the Mail window doesn't. :-(
*** Bug 63652 has been marked as a duplicate of this bug. ***
Comment 113•24 years ago
|
||
*** Bug 63652 has been marked as a duplicate of this bug. ***
Comment 114•24 years ago
|
||
(spam) Adding myself since I'm interested in seeing when this will be done for
MailNews
Reporter | ||
Comment 115•24 years ago
|
||
I just checked in a patch to make min/max state persistent on mail/news and
editor windows. Here's a patch to ensure new windows are opened to the state of
the frontmost window of their type. (It wasn't as complicated as I had feared).
On Dasher, On Comet, On Spam Machine! Yah!
Index: nsWebShellWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp,v
retrieving revision 1.323
diff -u -r1.323 nsWebShellWindow.cpp
--- nsWebShellWindow.cpp 2000/12/14 23:27:59 1.323
+++ nsWebShellWindow.cpp 2000/12/27 23:49:20
@@ -451,7 +451,15 @@
break;
}
case NS_MOUSE_ACTIVATE:{
+ // replace persistent size data with the newly activated window's
+ void* data;
+ aEvent->widget->GetClientData(data);
+ if (data) {
+ nsWebShellWindow *win = NS_REINTERPRET_CAST(nsWebShellWindow *,
data);
+ if (win->mChromeLoaded)
+ win->StoreBoundsToXUL(PR_TRUE, PR_TRUE, PR_TRUE);
+ }
break;
}
Comment 116•24 years ago
|
||
A patch for Mail/News? Yay!
Not to be greedy, but can we get that patch applied to the Bookmarks, email
Composition, and Address Book windows?
:-)
Reporter | ||
Comment 117•24 years ago
|
||
Casey: unless I goofed, those windows should now all persist their zoom state.
Note that the above patch (which by the way actually applies to bug 17149, which
this bug has grown to encompass) only works on one platform; there are
disappointing platform-specific issues about the reporting of window activation.
Needs more work.
Comment 118•24 years ago
|
||
Don't forget about Search Bookmarks/History and the History window.
Reporter | ||
Comment 119•24 years ago
|
||
Reporter | ||
Comment 120•24 years ago
|
||
Adding a comment to make this bug report larger.
PS - note the above patch is, unwisely, mostly removal of duplicated code. The
actual functional part is restricted to two lines, numbers 186 and 187.
Comment 121•24 years ago
|
||
r=saari
Comment 122•24 years ago
|
||
WinME (probably all win9x variants) Moz.2000123120:
Open two maximized windows, then minimize one of them. Working with the one
remaining maxed window, open a new window (CTRL-N, open link in new window,
whatever). This new window will not be maximized like the active window.
Netscape 4.x will open the new window maximized, and I'd hope that behavior is
carried over...
Anyway, I'm not real programming saavy, so bear with me.
danm, does the patch you posted (2000-12-29 14:35) address this?
If so; huzzah!
Reporter | ||
Comment 123•24 years ago
|
||
New-windows-in-frontmost-window's-state patch checked in. Kevin: yes, it'll fix
the problem you mentioned just above.
Comment 124•24 years ago
|
||
*** Bug 64348 has been marked as a duplicate of this bug. ***
Comment 125•24 years ago
|
||
I experience a weird behavior when I maximize a Mozilla 0.6 Browser window on Linux.
All new windows are created unmaximized but the same size as the maximized
window. With my window manager it's not easy to force the window different size
then since its borders are hidden.
Comment 126•24 years ago
|
||
The fix was not in 0.6. Please use nightly builds or wait for 0.7.
Comment 127•24 years ago
|
||
I have noticed that the maximize fix does not work for the Mail window, but it
does for all others now. Navigator, Address Book, Composer, Manage Bookmarks -
these will remember that they had previously been maximized, but not the Mail
window.
Reporter | ||
Comment 128•24 years ago
|
||
Casey: The main mail window (Tasks->Mail) and Message Compose work for me, but
the mail message window (individual window for reading mail) doesn't. Is that the
one you meant? That window probably just wants a one-line change to the xul.
Anybody know offhand which file that is? Blake: ditto for Search Bookmarks/
History and History.
hramrach: Linux builds don't know about min/max state; they remember only their
size. See my comments above, 2000-12-22 18:01.
Comment 129•24 years ago
|
||
do you mean messageWindow.xul
Comment 130•24 years ago
|
||
It does not work for me when I try Tasks >> Mail; I still have to maximize the
window manually. All other windows I can think of work just fine.
I have downloaded new builds almost every night, looking for this fix to take
effect, but it refuses to on my computer! :-(
Reporter | ||
Comment 131•24 years ago
|
||
timeless: well, that's an appropriately named window. Yup, that's it. Thanks!
This patch fixes that problem
Index: mailnews/base/resources/content/messageWindow.xul
===================================================================
RCS file: /cvsroot/mozilla/mailnews/base/resources/content/messageWindow.xul,v
retrieving revision 1.25
diff -r1.25 messageWindow.xul
44c44
< persist="width height screenX screenY"
---
> persist="width height screenX screenY sizemode"
Sigh. How many peoples' approval do I need to get this checked in?
I'm starting a discussion with caseyperkins offline, though I do enjoy modifying
this heavily watched bug report.
Comment 132•24 years ago
|
||
The Bookmarks/History search window is bm-find.xul
(xpfe/components/bookmarks/resources/), although it already seems to persist
window state. History is history.xul (xpfe/components/history/resources).
Comment 133•24 years ago
|
||
Ok, casey's right. If I max a window (nav, mail, composer, add book) then
minimize it and use the 'Tasks' menu on some other netscape window to bring it
back up the target window is no longer maximized - as it should be. This acts
kinda like the bug fixed with the "New-windows-in-frontmost-window's-state patch."
Footnote: This is observed with build 2001010804 (a day or so old).
Comment 134•24 years ago
|
||
Not only is the window "no longer maximized" - specifically, it is Restored, as
if you had clicked the Restore button on the title bar. I noticed this because I
had tiled my Address Book window with another program the other day, and after
trying what you said (opening the Address Book, minimizing it, and bringing it
back up via Tasks), it appeared in that Restored state. Same for the Composer
window.
That being said, what I originally had in mind was that the Mail window would
never come up Maximized no matter how I opened it, or whether or not I had
previously minimized it. Other windows would, though.
Comment 135•24 years ago
|
||
Now I tried 0.7 which behaves the same. I think it's possible to store the
previous window size or define a default window size on Linux.
(to allow 'unmaximizing' of windows that started in the 'maximized' size)
Reporter | ||
Comment 136•24 years ago
|
||
Blake: thanks for tracking those down. Both windows persist position and size,
though they do position incorrectly. I'll get to those.
Kevin: OK yes, I see that selecting a window from the Tasks menu resets its
zoom state. That kind of wants to be a new separate bug. I suppose we can leave
it as another piece of this ever-growing one, though.
We're still trying to figure out what's wrong with Casey's Mail window which
always refuses to persist its zoom state, no matter how you ask.
hramrach: The problem is that on Linux there is no general, window-manager
agnostic message stating that a window has been maximized. Or so I'm told.
Certainly nothing I know about. It's no problem noting and persisting a change in
size and/or position. In fact, that's what we're doing. The problem is realizing
that the window has been maximized, and we therefore should *not* persist its
size or position.
Comment 137•24 years ago
|
||
danm: I had a quick look at the new GNOME/KDE window manager spec, and to my
limited knowledge it looks like it has a way of doing this (
http://www.freedesktop.org/standards/wm-spec/ ). One would expect most window
managers will support this in the near future if they don't already.
Reporter | ||
Comment 138•24 years ago
|
||
Matthew: yes, you're right, we should probably support that spec. Alright, I'll
try hooking up the Linux build that way. It remains to be seen which window
managers actually send a _NET_WM_STATE event, but if those guys are staking out a
standard, we'll try to follow. *My* window manager is going to have to support
this stuff before I'll be able to actually check in a patch. I'll post a patch or
a capitulation once I figure that out.
With the recent move of timeless' 7000-line-diff cleanup effort to bug 65415,
this issue becomes the only thing I know of that keeps this bug open.
Comment 139•24 years ago
|
||
I still see this problem in mail when the mail window layout is:
-----
| | |
-----
| |
-----
that is, the top half has both folders and messages, and the bottom half is only
for message display. The file is mail3PaneWindowVertLayout.xul I believe.
Comment 140•24 years ago
|
||
I just fixed the vertical mail layout problem that kerz mention (thanks to kerz
for the fix.)
Comment 141•24 years ago
|
||
One more comment (sorry if already mentioned, but I am lost in this bug's
comments!):
From a maximized window, open a Javascript popup with specified width/height,
minimize it and restore it back. Then the popup window gets maximized! If the
"parent" window was not maximized, the popup retains its original dimensions.
For example try the review of Dreamweaver 4 on Builder.com on a maximized window:
http://home.cnet.com/webbuilding/0-7255-8-4535879-1.html?tag=st.bl.3880.pro_h.7255-8-4535879
Click on the "screenshots" link, minimize and restore the popup window.
My build: 2001-01-30-04 Win98
Comment 142•24 years ago
|
||
Can't see any problem with the popup using 2001021506 on Linux/icewm.
Comment 143•24 years ago
|
||
As I understand it, in Linux we cannon (so far) persist a maximized state, only
size and position of the window. That is probably why you don't observe the
problem with the popups in Linux. Still present in Windows 2001-02-14-04 build.
Updated•24 years ago
|
Target Milestone: mozilla0.9 → mozilla0.9.1
Comment 144•24 years ago
|
||
->moz0.9.1
Comment 145•24 years ago
|
||
From danm@netscape.com 2001-01-16 17:02:
(...) It remains to be seen which window managers actually send a _NET_WM_STATE
event, but if those guys are staking out a standard, we'll try to follow. *My*
window manager is going to have to support this stuff before I'll be able to
actually check in a patch. (...)
According to its authors Sawfish currently is 99% compliant so this shouldn't be
a problem. You can check the state of its support of the freedesktop's wm specs
on their bugzilla: http://bugzilla.eazel.com/show_bug.cgi?id=4268.
Comment 146•24 years ago
|
||
I don't think danm uses sawfish, but i do know that we have lots of sawfish
bugs. considering this bug is marked pc/nt should we use a new bug to track
sawfish/x11 managers?
Reporter | ||
Comment 147•24 years ago
|
||
I've recently moved to sawfish, with optimism. Haven't yet had a chance to look
at this problem, being swamped for 0.9. I don't feel a great need to open a new
bug for this one remaining facet of this bug. Besides, I love modifying this and
just sitting back and watching the spam it generates.
Comment 148•24 years ago
|
||
No time for us to fix this in the current release cycle. ->future/helpwanted
Keywords: helpwanted
Target Milestone: mozilla0.9.1 → Future
Comment 149•22 years ago
|
||
WFM on Windows XP with build 12/23. This has WFMed for over a year. Why is
this bug still open?
Comment 150•22 years ago
|
||
If this works for you, then mark it WFM...
Comment 151•22 years ago
|
||
The bug hasn't been fixed on Linux, see comment 108.
Comment 152•22 years ago
|
||
Not sure if this is related, but in 1.3a on Windows 2000, the window
restore/maximized is correct UNTIL I exit Mozilla and restart. If I close
Mozilla with a window maximized, then restart Mozilla, the 'restore' state of
the Window uses the full screen.
This was previously working correctly, ie. on restart it would remember the true
last restored window size.
Comment 153•22 years ago
|
||
Mozilla 1.3 seems to save maximized state, but it doesn't revert to normal state
when the window is unmaximized. New bug #198874 with that.
Comment 154•22 years ago
|
||
Confirming Ari's comment 153, I'm happy that 198874 will address my issue, and
that since he created it, others have reproduced same problem.
Updated•22 years ago
|
OS: Windows NT → All
Comment 155•20 years ago
|
||
This seems to be fixed in Firefox & gtk2. I vote for closing the bug.
Keywords: qawanted
Comment 156•18 years ago
|
||
In Firefox 2, the help window doesn't remember it's maximized state on Windows.
Comment 157•17 years ago
|
||
It's a pretty old bug and probably the original bug has been fixed, but in Firefox 2.0.0.9-3.1 (on opensuse 10.3/KDE 3), the maximised window state is not remembered. Once after an upgrade, it started with a smaller window and since then I can't get it to remember its maximised state.
Also, the window position is not remembered. If I change the window size, the size is remembered, but at restart it's always positioned to the left of the screen.
Comment 158•17 years ago
|
||
Removed localstore.rdf from the profile, and the problem is gone. I still have the old file, if anybody is interested in it...
Comment 159•17 years ago
|
||
Well, that conclusion was a little too swift. The problem with the maximised state is gone. But the position of a normal window is not honoured at startup, although it is saved in localstore.rdf:
<RDF:Description RDF:about="chrome://browser/content/browser.xul#main-window"
sizemode="normal"
width="839"
height="515"
screenX="374"
screenY="177" />
</RDF:RDF>
At startup, the normal window appears with the mentioned dimensions, but its top-left position is "0,0".
Comment 160•17 years ago
|
||
Edwin: on linux we don't actually restore the position of the window, instead relying on the window manager to do that for us. Not every window manager does that or is set up to do that, however. If you want to change that behavior, I suggest filing a new bug.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Keywords: helpwanted,
qawanted
You need to log in
before you can comment on or make changes to this bug.
Description
•