Closed
Bug 255270
Opened 20 years ago
Closed 20 years ago
[FIXr]Unchecking "Alert Me" in Security warning dialog hides Continue/Cancel buttons
Categories
(SeaMonkey :: General, defect, P1)
SeaMonkey
General
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.8alpha3
People
(Reporter: pkwarren, Assigned: bzbarsky)
References
Details
(Keywords: regression, smoketest)
Attachments
(2 files, 2 obsolete files)
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
(deleted),
patch
|
neil
:
review+
dbaron
:
superreview+
asa
:
approval1.8a3+
|
Details | Diff | Splinter Review |
Steps to reproduce
1) Create a new profile. Start Mozilla.
2) Go to any page which has a form (e.g. http://www.mozilla.org/).
3) Type something in the form, and submit it. This should bring up the security
warning dialog.
4) Uncheck the "Alert me" checkbox in the dialog. Notice how the label resizes
and pushes the Continue/Cancel buttons off screen so they are no longer visible.
This started occurring with nightly builds between 2004-08-09 and 2004-08-10.
Sorry if this is a duplicate - I didn't see this reported anywhere today.
Reporter | ||
Comment 1•20 years ago
|
||
I have reproduced this problem on Windows, Linux, and AIX.
Reporter | ||
Updated•20 years ago
|
Summary: Security warning dialog resizes and hides Continue/Cancel buttons → Unchecking "Alert Me" in Security warning dialog resizes and hides Continue/Cancel buttons
Reporter | ||
Updated•20 years ago
|
Summary: Unchecking "Alert Me" in Security warning dialog resizes and hides Continue/Cancel buttons → Unchecking "Alert Me" in Security warning dialog hides Continue/Cancel buttons
Assignee | ||
Comment 2•20 years ago
|
||
> between 2004-08-09 and 2004-08-10.
What actual builds? Morning? Evening?
Assignee | ||
Comment 3•20 years ago
|
||
Or more to the point, what are the timestamps on those builds?
Assignee | ||
Comment 4•20 years ago
|
||
If it's 08:00:00 to 08:00:00, this is probably a regression from bug 230170
(though I'm not sure why quite yet).
Reporter | ||
Comment 5•20 years ago
|
||
These are from nightly builds between 2004-08-09 08:00 and 2004-08-10 08:00,
Pacific time.
Assignee | ||
Comment 6•20 years ago
|
||
Short story: this is a known bug in nsBoxToBlockAdaptor. I have no idea why
we've suddenly started triggering it here....
Assignee | ||
Comment 7•20 years ago
|
||
I did a bit of digging about... Apparently, the change to the "collapsed"
attribute of the checkbox container in the dialog is being unhappy. If I make
sure to process the restyle synchronously for "collapsed" changes, things work
again.
By "is being unhappy" I mean that at some point we end up in a state where
GetBounds() on a BoxToBlockAdaptor gives a zero-width rect. Then the
incremental reflow spawned by the "checked" change (because _all_ attr changes
cause reflow in XUL!) starts with a width of 0 and ends up with the minimal
width it can have (which means putting each word on a separate line).
I tested opening up the prefs dialog to make sure the checkbox XBL binding is
preloaded, and that does not affect the problem at hand, so it looks to me like
this is a problem with XUL incremental reflow, pure and simple. Apparently the
reason this didn't happen before is that we changed the visibility in onload, so
the initial reflow subsumed our incremental style change reflow or something...
I can special-case the "collapsed" attribute if driven to it, but I think the
right thing to do is to try and hunt down this XUL reflow bug...
Comment 8•20 years ago
|
||
This is happening in alert dialogs with check boxes all over the app in trunk
builds. It's affecting firefox trunk too.
Severity: normal → critical
Comment 9•20 years ago
|
||
*** Bug 255058 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Keywords: regression
Assignee | ||
Updated•20 years ago
|
Severity: critical → major
Comment 10•20 years ago
|
||
Per Asa, made this a smoketest blocker and blocking1.8a3
Assignee | ||
Comment 11•20 years ago
|
||
Note that this does not use collapsed or anything. It may be that the reason
the "collapsed" thing helps is that it marks descendants dirty....
Assignee | ||
Comment 12•20 years ago
|
||
Assignee | ||
Comment 13•20 years ago
|
||
Attachment #155966 -
Attachment is obsolete: true
Attachment #155972 -
Attachment is obsolete: true
Assignee | ||
Comment 14•20 years ago
|
||
Assignee | ||
Comment 15•20 years ago
|
||
Comment on attachment 155988 [details] [diff] [review]
Proposed patch
David, the short story is that the !needsReflow codepath doesn't do a reflow
and never sets aDesiredSize.mMaximumWidth. Later on, if we have
NS_REFLOW_CALC_MAX_WIDTH set, we use aDesiredSize.mMaximumWidth as our
preferred width. So we come out with a preferred width of zero on incremental
reflow, and then end up using our minimum width...
The other option is to cache the "last used" mMaximumWidth and use it when
needsReflow is false. But I'm not sure that wouldn't leave in other
incremental reflow bugs...
The nsHelperAppDlg.xul parts of the patch are just removing hacks that were
added to sort of work around bug 189076 (which this patch fixes).
Attachment #155988 -
Flags: superreview?(dbaron)
Attachment #155988 -
Flags: review?(dbaron)
Assignee | ||
Updated•20 years ago
|
Assignee: general → bzbarsky
Blocks: 189076
Priority: -- → P1
Summary: Unchecking "Alert Me" in Security warning dialog hides Continue/Cancel buttons → [FIX]Unchecking "Alert Me" in Security warning dialog hides Continue/Cancel buttons
Target Milestone: --- → mozilla1.8alpha3
Assignee | ||
Comment 16•20 years ago
|
||
*** Bug 255455 has been marked as a duplicate of this bug. ***
Comment 17•20 years ago
|
||
Comment on attachment 155988 [details] [diff] [review]
Proposed patch
r+sr=dbaron, although you might want someone else to review the xul change
Attachment #155988 -
Flags: superreview?(dbaron)
Attachment #155988 -
Flags: superreview+
Attachment #155988 -
Flags: review?(dbaron)
Attachment #155988 -
Flags: review+
Assignee | ||
Comment 18•20 years ago
|
||
Comment on attachment 155988 [details] [diff] [review]
Proposed patch
Neil, could you review the XUL changes?
Could this please be approved for 1.8a3?
Also, I probably can't check this in until Monday.
Attachment #155988 -
Flags: review?(neil.parkwaycc.co.uk)
Attachment #155988 -
Flags: review+
Attachment #155988 -
Flags: approval1.8a3?
Updated•20 years ago
|
Attachment #155988 -
Flags: review?(neil.parkwaycc.co.uk) → review+
Assignee | ||
Updated•20 years ago
|
Summary: [FIX]Unchecking "Alert Me" in Security warning dialog hides Continue/Cancel buttons → [FIXr]Unchecking "Alert Me" in Security warning dialog hides Continue/Cancel buttons
Comment 19•20 years ago
|
||
Comment on attachment 155988 [details] [diff] [review]
Proposed patch
>--- embedding/components/ui/helperAppDlg/nsHelperAppDlg.xul 18 Apr 2004 22:00:48 -0000 1.23
>+++ embedding/components/ui/helperAppDlg/nsHelperAppDlg.xul 12 Aug 2004 23:26:22 -0000
>@@ -68,24 +68,19 @@
> <image id="contentTypeImage"/>
> </vbox>
> </hbox>
>
> <separator orient="horizontal" class="thin"/>
>
> <radiogroup id="mode" oncommand="dialog.toggleChoice()" align="stretch">
> <hbox>
>- <!-- XXX Wallpaper over incremental reflow bug in XUL by giving
>- ourselves more space... In fact, all this business
>- with the hboxes is wallpaper. -->
> <radio id="useSystemDefault"
>- flex="7"
> label="&useSystemDefault.noDesc.label;"
> accesskey="&useSystemDefault.accesskey;"/>
>- <spacer flex="3"/>
> </hbox>
I'd advise against getting rid of the spacer altogether, radiobuttons look kind
of bad on gtk at least if you let them flex horizontally as much as they want
(since the hover effect is on the whole button + label area). I'd recommend
just restoring this to how it was before you added the wallpaper:
<radio id="useSystemDefault"
flex="1"
label="&useSystemDefault.noDesc.label;"
accesskey="&useSystemDefault.accesskey;"/>
<spacer flex="1"/>
Comment 20•20 years ago
|
||
Comment on attachment 155988 [details] [diff] [review]
Proposed patch
<bryner> Asa: does bz need approval for his smoketest blocker fix?
http://bugzilla.mozilla.org/show_bug.cgi?id=255270
<Asa> no
<Asa> a=asa
Attachment #155988 -
Flags: approval1.8a3? → approval1.8a3+
Comment 21•20 years ago
|
||
I checked this in so we can get the tree open. I just restored the
nsHelperAppDlg.xul code to its previous state as I said in comment 19... feel
free to change it if you want to later.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 22•20 years ago
|
||
>>- flex="7"
>I'd advise against getting rid of the spacer altogether, radiobuttons look kind
>of bad on gtk at least if you let them flex horizontally as much as they want
Except he isn't letting it flex as much as it wants, is he...
Assignee | ||
Comment 23•20 years ago
|
||
Comment on attachment 155988 [details] [diff] [review]
Proposed patch
Brian, I do know why that code was there, and I do know what I'm doing in
removing it, as Neil pointed out.
Note that the change _you_ made actually makes the rendering of the helper app
dialog worse.
Re-requesting approval for the patch as it should go in...
Attachment #155988 -
Flags: approval1.8a3+ → approval1.8a3?
Assignee | ||
Comment 24•20 years ago
|
||
*** Bug 255352 has been marked as a duplicate of this bug. ***
Comment 25•20 years ago
|
||
Comment on attachment 155988 [details] [diff] [review]
Proposed patch
I approved this online.
Attachment #155988 -
Flags: approval1.8a3? → approval1.8a3+
Comment 26•20 years ago
|
||
I updated nsHelperAppDlg.xul as per the original patch.
*** Bug 255944 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 28•20 years ago
|
||
*** Bug 256352 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 29•15 years ago
|
||
The "security warning dialog" before submitting something to the web is gone in Seamonkey 2. VERIFIED
Status: RESOLVED → VERIFIED
Comment 30•15 years ago
|
||
(In reply to comment #29)
> The "security warning dialog" before submitting something to the web is gone in
> Seamonkey 2. VERIFIED
Actually it isn't, but it's off by default. Maybe we should enable show_once?
You need to log in
before you can comment on or make changes to this bug.
Description
•