152752 - Ensure all text fits and displays in dialog sheets

Status: VERIFIED FIXED

(Camino Graveyard :: General, defect, P3)

Description

[Peter Scholtz]

Chimera 0.30
Mac OS X 10.1.5

Steps to reproduce:
(1)Go to above URL
(2)Click on "Buying a Home" link (upper middle of page)
(3)fill out the form on the next page any way you like
(4)click Continue
(5)click the Back button of the browser

You'll see the alert that is so long it is clipped.  What would be the fix for this?

Comment 1

[Greg K.]

Confirmed using Chimera/20020617. Back button security warning text ("For your
security, the browser's back button is not able to save the information you have
entered up to this point....") is too large for dialog sheet and is clipped.
Related to bug 150732, but broader.

Comment 2

[Steve Smart]

Attachment: Extremely long JavaScript alert testcase

We will need something to dynamically determine what size sheets created for
JavaScript should be. It will need to try to keep the proportion of the dialog
box the same until it reaches the full width of the screen, then just expand 
the other way until it uses all availible screen height, then it will have to
make the sheet scrollable. This will also have to be applied when the alert is
displayed as a utility window when the tab is in the background.

I don't know if we want to bother being that thorough right now, but I suppose
all the bugs related to this show that authors can and do put huge amounts of
data in JavaScript alerts, and a system like that could handle any amount of
text.

Attached a testcase of a link that opens a JavaScript dialog with the first
three chapters of A Tale of Two Cities. IE is the only other browser I found
that can handle this testcase correctly (creates scrollbars). Moz, NS, and Omni
create a sheet that is larger than the screen, preventing you from seeing the
bottom of the alert or the OK button; we should make sure our function doesn't
do that.

Comment 3

[saari (gone)]

cc'ing rjc in the hope that he'll have a good suggestion

Comment 4

[Robert John Churchill]

A sheet on Mac OS X (whether Mozilla or Chimera) is really just a XUL dialog
(which are intrinsically sized based upon their content.)

I guess that I am surprised that dialogs don't have a maximum width/height
enforced.  Anyway, doing that would probably give the desired effect.  Shame
that danm is on sabbatical, he's know the full story.

Comment 5

[Mike Pinkerton (not reading bugmail)]

rjc, this is chimera, not seamonkey

Comment 6

[saari (gone)]

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

Comment 7

[Robert John Churchill]

pink:  This bug happens in SeaMonkey as well as Chimera.

Comment 8

[Steve Smart]

Well, it's a different bug in Moz than in Chimera. In Mozilla all the text is
dispayed in the sheet, but it is too large and goes off the screen; Chimera
displays a standard sized sheeet and truncates the message. This bug is for the
problem with Chimera not displaying all the text; I don't know if there is one
for Moz. Feel free to take this testcase for that bug as well.

Comment 9

[Winnie Lam]

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

Comment 10

[Greg K.]

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

Comment 11

[Greg K.]

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

Comment 12

[sairuh (rarely reading bugmail)]

i ran into this while running through some structured url parsing tests for
security.  the results are at

http://hopey.mcom.com/tests/security/buffer-overflow/results-urlParsing-chimera.html

Comment 13

[Richard Schreyer]

I figured out how to properly size a window based on the amount of text in it
when I was fixing the <a
href="http://bugzilla.mozilla.org/show_bug.cgi?id=149635">tooltips bug</a>.  Is
anyone actively working on this one right now?  If not, I think I'll take it.

This is a fairly important internationalization issue too.  A lot of languages
are not as compact as english, and you can't say much with a default-sized
dialog box.

Comment 14

[Stephane Moureau]

In the mean time if you could enlarge (4+ lines) the static text field for the
javascript alert command, that would be great, because some sites are really
showing a novel in this dialog.
I don't think it's possible, perhaps allow a dynamic resize of the sheet/dialog,
with a grow box.
Keep up the good work, thanks for making such a great browser.

Comment 15

[Mike Pinkerton (not reading bugmail)]

by all means, take it ;)

Comment 16

[Richard Schreyer]

I've been thinking about some UI issues caused by using sheets instead of app
modal dialogs for these things.

While sheets are nifty, and the expected OS X way of doing things, they were
designed for one-document-per-window designs, rather than multiple docs per
window.  Right now, if you type in a bad address, and switch tabs, that other
page will fail silently in the background.  Even if we did fix that and bring up
a sheet, what if there were already a sheet attached to the window?

This line of thinking leads me to believe that while sheets would work fine if
we were not using tabbed browsing, we should probably switch to using app modal
dialogs until we can come up with some kind of tab-modal alert in the future.

Any comments?

Comment 17

[Richard Schreyer]

Ugh, back to this boring thing again ;)

I've been thinking of how the panel should be sized...  Does the web developer
ever have the ability to set the title of any of these dialogs, or do they have
the ability to set the checkbox text (when the checkbox is there)?

If not, it gets a lot easier, since then we can just hardcode some maximum size
for these elements that is larger than what mozilla will ever put up (1 or 2
lines for the title, perhaps 3 or 4 for the checkbox?)

Comment 18

[Mike Pinkerton (not reading bugmail)]

i think the web developer can set the title, but since it's a sheet, we simply
ignore it. as far as hardcoding a size, how would that work with localizations?

Comment 19

[Richard Schreyer]

By hardcoding a size, I guess I really meant hardcoding a maximum size for these
fields, something like 5 or 6 lines.  They can be smaller if necessary.

If they're not tall enough for some common case, it will be pretty easy to tweak
later.  I ask because it makes the job a lot simpler of the message text is the
only bit that must be of 'infinite' height (ie, placed in a scroller)

Comment 20

[Mike Pinkerton (not reading bugmail)]

i'm not certain if a website can show a prompt with a checkbox. maybe it can.
regarless we need to be able to show all the main text of the prompt that comes
from a website.

Comment 21

[Benoit]

If the following links contain up to date Javascript reference:

  http://developer.netscape.com/docs/manuals/js/client/jsref/window.htm#1203172
  http://developer.netscape.com/docs/manuals/js/client/jsref/window.htm#1201916

I don't think it's possible. You can only set the text and initial text if a
second argument is specified in javascript:prompt(). 

I think the code that handles this is in dom/src/base/nsGlobalWindow.cpp in the
method GlobalWindowImpl::Prompt(nsAString& aReturn). It looks like gecko
actually supports more that 2 arguments for javascript:prompt(), the third one
to set the title and the fouth for the checkbox... however, the checkbox never
seems to show up because the nsIAuthPrompt::Prompt() method which display the
prompt is always called with a null passwordRealm argument...

In short if the links are up-to-date I don't think the web developper can set
the title or the checkbox message. I'm not sure what is using this prompt in
Gecko though... a grep on "Prompt(" in the code seems to indicate that only the
javascript prompt and the mailnews stuff is using it...

Comment 22

[Richard Schreyer]

Great, then that should make this a bit simpler.  It shouldn't take me too much
longer to finish this up.  I have most of the dirty work done.  I decided to
dump the alert nibs completely, since they just weren't flexible enough when you
have a bunch of elements that are always resizing themselves.  It was easier
just to code the dialogs completely by hand.

I've come up with a method that will create and properly size a dialog given the
titles of it's 3 buttons, the dialog title, message, and an optional additional
NSView.  The extra view contains things like a checkbox + label, input fields,
etc (since this will be specific to each kind of dialog).

In the meantime, I've got a couple screenshots of the new dialogs.  Work on the
rest (prompts, authentication, etc) should go quite fast from this point, since
I just have to throw together the 'extra views' for these.

http://homepage.mac.com/frogblast/other/security_dialog.jpg
http://homepage.mac.com/frogblast/other/big_dialog.jpg

Comment 23

[Richard Schreyer]

Attachment: First shot at fixing problems with the dialog code

Here's the initial patch.  It's pretty much feature complete, although there
are a few nagging issues that need to be worked out before it's ready to be
landed.  alert.nib is now not used at all.

One question, why is Localizable.strings marked as a binary file in CVS?  I had
to attach the new version separately.

Comment 24

[Richard Schreyer]

Attachment: Fixed patch after the file reorg

Ok, no significant changes, it just works after the file re-org.  I need some
brave volunteers to run with this one for awhile to look for bugs.  Any
volunteers?

Comment 25

[Simon Fraser [no longer active]]

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

Comment 26

[Marko Karppinen]

I see the proverbial slippery slope here. Tabbed Browsing isn't a good reason to
abandon the established human interface elsewhere. Document-modal dialogs should
be implemented as sheets in Mac OS X. If alert() and prompt() are not
document-modal, I don't know what is.

For displaying a sheet on a tab, I'd first check to see if the tab was visible.
If yes, I would just display the sheet; if not, I would change the tab title to
indicate that something's up (much like the "Loading..." title does now), and
"queue" the sheet to be shown when the obscured tab again gains focus.

As these are document-modal dialogs, I don't see a problem with no actual alerts
being made of obscured sheets.

Comment 27

[Greg K.]

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

Comment 28

[Greg K.]

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

Comment 29

[John Unruh]

The product is going to be released without this being fixed? All of the 
SSL security warning dialogs are cut off.

Comment 30

[kaldari]

Please do not implement JavaScript dialogs as app modal
dialogs rather than sheets. Sheets are the right thing to do here, they just
need to be a little smarter. I've been writing JavaScript programs for 6 years
and the longest JavaScript dialog I've ever needed was 10 lines long. If someone
claims they need to put Tale of Two Cities in a JavaScript alert, they should be
shot. Anything longer than 10 lines should be moved to an HTML page. Just my 2
cents. Sorry for the spam.

Comment 31

[Steve Smart]

JavaScript sheets in background tabs should behave exactly the same way site not
found or any other number of alert sheets behave in background tabs; that
discussion should be held in a seperate bug so that all our sheets behave
consistantly.

Comment 32

[sairuh (rarely reading bugmail)]

nominating...

Comment 33

[Greg K.]

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

Comment 34

[Mike Pinkerton (not reading bugmail)]

has anyone tried running with this patch?

Comment 35

[Simon Fraser [no longer active]]

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

Comment 36

[Matt Carrell]

I agree with comments #26 (Marko) and #30 (kaldari)... If we don't use sheets and opt to 
create more tabs or javascript dialogs, then what's the point of Aqua?  Simply creating 
another tab for alerts or opening a javascript alert doesn't tell us which tab in multi-tab 
browsing was responsible for the alert, and its messier.. and easier to ignore or 
overlook..  Remember, an alert could come from a page loading in the background.. as 
tends to happen with users who are multitasking prone.. in that case, you must use 
sheets in order to direct the user to the proper associated tab.. Let's keep with the 
wisdom of the established standards please.. :)  

Comment 37

[Simon Fraser [no longer active]]

Patch needs to be diff -u. Localized.strings is binary because it's Unicode with
a BOM

Comment 38

[Matt Carrell]

Are you saying a method of sheeting for each tab can't be created?  Surely if
there isn't an OS-approved way of attaching sheets to tabs instead of to the
overarching browser window, a way of similating a sheet just for each tab could
be created to keep the UI approach constant.

If not, how about attaching a visual cue to the sheet..  When the sheet
displays, it highlights the tab for which it was meant in aqua or something that
focuses attention to the specific tab so it's not questionable if sheets become
layered like you describe?

Comment 39

[Mike Pinkerton (not reading bugmail)]

patch applied and cleaned up. fixed. the one issue that i can't find an easy way
to solve is that now clicking the text of a checkbox doesn't toggle the
checkbox. we can clean that up later.

QA: this will require retesting anything that put up a dialog or alerts. JS
alerts, JS prompts, login dialogs (both username/password and just password
varieties), cookie prompts, networking errors, POST errors, etc.

i tested all the things listed above and they worked fine, as do the case in
comment #0 and the first attachment of a very long JS dialog.

Comment 40

[Chris Petersen]

Marking verified in the 2003-01-14-04 NB under 10.2.3. Testing was based on
pink's suggestions in his last comments. JS Alerts, FTP user/password, Logins,
keychain sheet, popup blocking sheets, cookie alert sheets, and https security
sheets were tested.

Comment 41

[mkosnik+bugzilla]

Attachment: screen shot of sheet.

Example of sheet that is not big enough to contain text.

Comment 42

[mkosnik+bugzilla]

Case of still broken:
Build 2003011304

Citibank.com - javascript? sheet confirming amount to be paid (bill pay portion
of site)...  The whole lower portion is clipped (screen shot in attachment 
Attachment #111618 [details])

Comment 43

[Mike Pinkerton (not reading bugmail)]

given that i checked the code in on 1/13, the 1/13 build would not have my fix
and of course it would still be broken.

try a newer build.

Comment 44

[mkosnik+bugzilla]

Attachment: letter bottoms being cropped.

using build: 2003011804

The bottom of the p and q are cut off on the bottom line of the text box.