Closed Bug 379726 Opened 18 years ago Closed 18 years ago

[10.4] Javascript alert sheets don't open if Camino is hidden

Categories

(Camino Graveyard :: OS Integration, defect)

Product:
Camino Graveyard
Component:
OS Integration
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Camino1.6

People

(Reporter: scott, Assigned: murph)

Details

(Keywords: fixed1.8.1.5, Whiteboard: [camino1.5.1])

Attachments

(2 files, 1 obsolete file)

test case
176 bytes, text/html
Details
fix which doesn't reopen another bug
1.42 KB, patch
nick.kreeger
: review+
alqahira
: review+
stuart.morgan+bugzilla
: superreview+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/419 (KHTML, like Gecko) Safari/419.3 Build Identifier: Version 2007022813 (1.0.4) If Camino is hidden when javascript fires an alert, the alert sheet doesn't get drawn, even when Camino is activated and the window comes forward. You can test it with the following HTML doc. Put that HTML into a file, load the file into Camino and quickly hide the application (cmd-H). 5 seconds after page load, the javascript will call alert. Camino will then bounce the dock icon correctly. Bring Camino forward and you now have what appears to be a hung camino window. The UI doesn't respond, you can't quit camino, ... The alert is active, but just not visible. Hitting return dismisses the invisible alert sheet and things go back to normal. <html> <head> <title>Hidden alert test page</title> <script language="javascript"> setTimeout("alert('Testing')", 5000); </script> </head> <body> This is a page </body> </html> Reproducible: Always Steps to Reproduce: 1. Load page with javascript that calls alert() after some time 2. Hide Camino Actual Results: Alert gets called, but the alert sheet isn't drawn. Camino then appears hung but it's just waiting for you to click the invisible "ok" button or hit return Expected Results: When Camino is brought forward, I should see a window with an attached alert sheet
This is almost exactly what we see in bug 354643, so I bet it's a dupe of whatever underlying problem causes that.
Actually, I can't trigger this to appear hidden in 1.5rc. Clicking on the Dock icon, or cmd-tabbing to Camino always shows the alert dialogue.
I can't reproduce this in my 1.0.x build, either; the alert window always shows. Scott, can you test the Camino 1.5rc and see if you can reproduce this there? http://ftp.mozilla.org/pub/mozilla.org/camino/nightly/2007-05-01-12-1.5/ Can anyone else trigger this hidden?
Attached file test case —
test case from comment 0
I can repro this. 1. loaded the testcase over http (first tested from my local dev server) 2. immediately Cmd+H 3. after 5 seconds, Camino's icon bounces 4. Cmd+tab to bring Camino forward (or click dock icon) 5. The main window is like half greyed out, the alert box invisible - no alert sheet. The whole interface is disabled. At this point: Cmd+H to hide Camino. The alert box becomes visible as a window. It is not hidden. Kind of the same effect as bug 354643. Force reload to repeat the test. Tested with latest trunk.
Camino 1.5 rc fails the same way here, as does Camino 1.04 - on 10.4.9 ppc. But the same browsers work fine on 10.3.9.
(In reply to comment #6) > But the same browsers work fine on 10.3.9. Good, I thought I was going crazy :) Since this is worse than bug 354643 (it can happen at any time), it'd be good to see if we can get a fix in a security update.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: camino1.5.1?
Summary: Javascript alert sheets don't open if Camino is hidden → [10.4] Javascript alert sheets don't open if Camino is hidden
Attached patch fix (obsolete) — — Splinter Review
As you may have noticed, sometimes javascript alerts do not appear in the usual sheet form and are displayed using an alert dialog. This happens because: > // If there is already a modal window up, convert a sheet into a modal window, > // because AppKit will hang if you try to do this (possibly because we're using > // the deprecated and sucky runModalForWindow:relativeToWindow:). > // Also, if the parent window already has an attached sheet, or is not visible, > // also null out the parent and show this as a modal dialog. The "is not visible" part is what concerns this bug. Documentation is no longer available for |runModalForWindow:relativeToWindow:|, but the visible status of the relative NSWindow should not matter. Further testing indicated that, when the application is hidden, |runModalForWindow:relativeToWindow:| behaves identically to more commonly used and non-depreciated alerting techniques from the user's point of view. Bug 221730 is responsible for replacing this flaky and very depreciated method entirely. 10.3.9 users might want to test out the patch to make sure the isVisible status does not matter there as well.
Assignee: nobody → murph
Status: NEW → ASSIGNED
Attachment #264019 - Flags: review?
(In reply to comment #8) > the visible status of the relative NSWindow should not matter. And how are you dealing with bug 325193, which is the reason the code you removed exists?
(In reply to comment #9) The isVisible check was introduced to prevent situations like the following: A site attempts to open a URL in a new window using javascript, but the URL is invalid. The invalid URL alert is displayed before the new window is ever onscreen. This new window is deferred and the window server has not yet created a window device for it, which is performed later when the window moves onscreen. My guess is that attaching sheets without an initialized window device is the cause of bug 325193; the visible status only serving as an implicit indicator of that. I was led to remove the isVisible check based upon what it was explicitly verifying. Regardless, I'll try for something better. Interestingly, if I create a NSWindow and pass that on to [NSApp runModalForWindow:] this bug does not happen. Supplying a NSPanel to runModalForWindow: is not incorrect, AFAIK. I'm looking for something we specifically change about the NSPanel instance created by |getAlertPanelWithTitle…| which might cause this.
Attachment #264019 - Flags: review?
I stumbled upon what I believe is the root cause of the misplacing panel trouble. As I mentioned, the bug did not occur if I changed around |getAlertPanelWithTitle:…| to return a NSWindow rather than NSPanel. Based upon this and using elimination, I looked at each attribute that makes a NSPanel subclass different… hidesOnDeactivate was responsible, and disabling it fixes the problem. This is the way Safari displays a javascript alert (a non-hiding modal dialog). Besides, I am not fan of hiding modal alert dialogs when the parent application is inactive, because if they're important enough to be modal in the first place they should be easily visible and not suddenly appear out of nowhere when the application becomes back into view. I might as well report this to Apple; it seems like a potential AppKit bug. NSWindows, let alone modal dialogs, that are hidden on deactivation are supposed to be restored when the application becomes active.
Attachment #264019 - Attachment is obsolete: true
Attachment #264276 - Flags: review?
Attachment #264276 - Flags: review? → review?(nick.kreeger)
Comment on attachment 264276 [details] [diff] [review] fix which doesn't reopen another bug This seems to prevent the application lockup. I can only test on 10.4, so someone running 10.3.9 should give this a run. r=me
Attachment #264276 - Flags: review?(nick.kreeger) → review+
Comment on attachment 264276 [details] [diff] [review] fix which doesn't reopen another bug This preserves existing behavior on 10.3.9 on branch (as near as I can tell). On the trunk, it forces the app to unhide when the sheet appears. However, since 10.3 is going to die on the trunk at any moment, I think we need not worry about that part.
Attachment #264276 - Flags: review+
Comment on attachment 264276 [details] [diff] [review] fix which doesn't reopen another bug I think smorgan volunteered to sr this...
Attachment #264276 - Flags: superreview?(stuart.morgan)
Comment on attachment 264276 [details] [diff] [review] fix which doesn't reopen another bug sr=smorgan
Attachment #264276 - Flags: superreview?(stuart.morgan) → superreview+
Landed on trunk and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1.5
Resolution: --- → FIXED
Whiteboard: [needs checkin]
Target Milestone: --- → Camino1.6
Landed on CAMINO_1_5_BRANCH.
Flags: camino1.5.1? → camino1.5.1+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: