Closed Bug 424266 Opened 15 years ago Closed 15 years ago

"Default browser" or proxy auth dialog (sheet) appears mostly off-screen

Categories

(Core :: Widget: Cocoa, defect, P2)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: jruderman, Assigned: smichaud)

References

Details

(Keywords: regression)

Attachments

(3 files)

Attached image screenshot
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008032016 Minefield/3.0b5pre

Steps to reproduce:

1. Create a new profile OR check the last box in Options > Advanced > General ("Always check to see if Minefield is the default browser on startup".

2. Relaunch Firefox (from the command line or from the dock).

Result: 

A dialog appears near the top-left of my screen asking whether I want to make Firefox my default browser.  Much of the dialog, including the checkbox for making the dialog not appear next time, is off-screen.
Flags: blocking-firefox3?
This is opening as a sheet, this shouldn't be modal... 
Assignee: nobody → mconnor
Flags: blocking-firefox3? → blocking-firefox3+
Duplicate of this bug: 424486
From the dup: this appeared some time between beta 4 and March 16.

This should probably block beta 5.
Keywords: regression
Not a b3 blocker, no.
Priority: -- → P2
Target Milestone: --- → Firefox 3
Hmm, I wonder if the regression range is correct since this looks like bug 356742 (cocoa regression) to me. Haven't seen this in Minefield before, though. But maybe something have changed so that the sheet launches before the main window is known (so the sheet launches from the hidden window?).

I just checked bug 356742 with a Tiger machine, and then I couldn't even see the sheet. However, iirc when I checked on Leopard for some time ago, it seemed to work. Hmm.
Its not opening from the hidden window, that's the really screwy thing.  There's no reason for it to have a parent, in any case, I thought we'd fixed that a long time ago...
QA: need a precise regression range so we can figure out what changed here.  Can confirm that this regressed since b4.
Keywords: qawanted
(In reply to comment #7)
> QA: need a precise regression range so we can figure out what changed here. 
> Can confirm that this regressed since b4.
> 

This regressed between: 2008031004 and 2008031104

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008031004 Minefield/3.0b5pre is fine

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008031104 Minefield/3.0b5pre shows the Problem from Jesse's Screenshot

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=20080310+04%3A00%3A00&maxdate=20080311+04%3A00%3A00&cvsroot=%2Fcvsroot
Per the dupe, the proxy auth sheet can also cause this, which is probably more common than the case Jesse and I reported (though still not an every-user case).
If you're behind the corp proxy-auth its pretty much death for FF3 usage at your corporation.
FWIW, I rebuilt my browser/ tree tonight, and there I see the "stuck mostly off-screen" behavior in Jesse's screenshot instead of the "sheet flies entirely back on-screen" behavior I see in official nightlies (including today's).
Summary: "Default browser" dialog appears mostly off-screen → "Default browser" or proxy auth dialog (sheet) appears mostly off-screen
Another clue:

Whatever answer you give to the "Default Browser" dialog (yes or no),
the main menu is truncated as Minefield starts up (it contains only
the Minefield submenu).  This problem has exactly the same regression
range as the main problem (as reported by Tomcat in comment #8).

It looks like the "Default Browser" dialog (which was and still is a
Gecko-modal dialog) is now appearing earlier than it used to.  Before
the 2008-03-11-04-trunk nightly it appeared after a browser window was
displayed, now it appears beforehand.

Maybe we should restore the previous timing.  Or maybe we should stop
the "Default Browser" dialog from being modal.  I'd vote for the
former.
I think that the initial readme dialog of the foxclocks extension suffers from the same problem.
I think that if you have dialog that normally opens as a sheet (modal), that dialog should not open as a sheet if there is no parent window. Iirc we used to do this before the switch to cocoa. In other words, I think the real solution to this can be found somewhere in cocoa-land. You can always fix the "Default Browser" dialog, but there are probably many more cases (as comment #15 indicates) out there.
FWIW, every now and then, I actually see the hidden window's menu come up before the sheet appears attached to a phantom window that eats the menus.  That is, launch, see the full set of menus (but no browser window yet), and then the crazy sheet appears.

Is another possible solution to make sure that you have a (real) window before showing a sheet?
> Is another possible solution to make sure that you have a (real) window before
> showing a sheet?

Yeah, that's basically what I suggested - with the addition of showing a dialog if you don't have a (real) window. 

fwiw, I'm pretty sure that this bug is the same as bug 356742, which I filed for 1.5 years ago as a cocoa regression... The difference here seems to be that the sheet opens (most of the time) before any hidden window. Perhaps this is also the reason why you end up with a menubar with just an application menu on Leopard. 

Yeah, I guess that's because the sheet/dialog doesn't have a menubar of it's own.
The Browser window popped first (otherwise I'd only have the auth proxy login sheet as per pictures on my duped bug) and the Borwser window definitely has a full menu bar.
> menu isn't fully initialized

This is by design -- the proxy auth dialog is modal.

After the dialog closes the browser window should once again have a full menu (as perhaps your comment #21 indicates).
Since pop up context menus in text boxes have been broken from Firefox 1.5 onwards (Bug #426596) it sure would be useful if someone would change the design to include Edit so a user could copy/paste the username/password into the proxy auth login sheet.
Duplicate of this bug: 427767
Duplicate of this bug: 427895
I've never seen the default browser sheet appear partially off screen, but it does appear without menus all the time since upgrading from Beta 4 to Beta 5.  What I see exactly mimics this movie that Samuel Sidler posted to bug 424486:

http://people.mozilla.org/~ss/ibugyou.mov

I can confirm the regression window though...I tested it myself with the builds mentioned in comment 8 and I see the problem in the 2008-03-11 build but not the 2008-03-10 build.  Also, I see the default browser dialog very frequently because of a FileVault-related glitch that resets my default browser (and other app) associations annoyingly often.
Huh...using the 2008-04-09 trunk build I DO see the default browser appearing mostly off-screen...it consistently happens when I launch the 2008-04-09 build, but consistently DOESN'T happen when I launch the 2008-03-11 build (though the other symptoms do).  So perhaps there was another checkin somewhere between there that made things worse?
Assignee: mconnor → enndeakin
See also bug 429119, "menu bar missing if Firefox is not default browser", which seems to happen whenever this bug happens.
Or just read bug 424486 (duped here), where I reported that same issue (with video), or comment 26 of this bug where Tom reported it. I don't think that's a "bug", however, since the is a modal sheet. See comment 22.
Even after I dismiss the "default browser" dialog and a browser window appears, the menus are missing.  Switching to another app and back makes the menus finally appear.  Definitely buggy behavior.
Yeah, you need to open another non-browser window (I always use prefs, because Cmd-, worked) to make Minefield "re-initialize" the menubar.  I'd mark bug 429119 as at least a depends, if not an outight dupe.  Everyone/bug who sees this one also sees that one.
(In reply to comment #31)
> Yeah, you need to open another non-browser window (I always use prefs, because
> Cmd-, worked) to make Minefield "re-initialize" the menubar.  I'd mark bug
> 429119 as at least a depends, if not an outight dupe.  Everyone/bug who sees
> this one also sees that one.
> 

I think there is overlap here, but is it an outright dupe?  The missing menu bar items behaviour was present in beta 5 but the default browser dialog/sheet didn't appear off the screen in this build.
The dialog is appearing offscreen because the parent browser isn't visible yet, so  the 'centerscreen' feature is trying to center the dialog at the upperleft corner, because it thinks the position/width/height of the parent are there.

Bug 401155 is the only one that seems suspicious on that regression range, although it was backed out.

The other issue is that the menu isn't being applied to the parent menu. That's likely a Cocoa issue, and possibly a different bug.

Making the dialog non-modal (a non-sheet) fixes the positioning and menubar issue.
(In reply to comment #33)
> The dialog is appearing offscreen because the parent browser isn't visible yet

FWIW, at least some of the time I see the browser window come up, then vanish and the sheet fly in from outer space.  Is that just a drawing race condition?
Looks like the timer for delayedStartup is running earlier than it used to, causing the default browser check to be done before the window has been shown. Can't figure out which bug caused it though -- that would be very helpful.

I've stumbled across a fix (maybe _the_ fix) for this.

I'm looking into it and will post more soon.
Attached patch Quick fixSplinter Review
Well, this probably isn't _the_ fix.  But it's simple and (as far as I
can tell) safe, and will do until we can come up with something more
fundamental.

This patch removes a workaround for a couple of problems that no
longer exist:

1) Versions of Mac OS X older than 10.4 couldn't deal with menu item
   icons changing while the menu is open in tracking.

2) Before the patches for bug 338225 and bug 389931 landed, Gecko
   events didn't get processed (and menu icons didn't get loaded)
   while MenuSelect is tracking the menu.

This workaround caused nsMenuItemIconX::LoadIcon() to spin a Gecko
event loop (to process Gecko events, causing menu icons to be loaded
synchronously) if the menu "comes from a local scheme".

But now (as of the 2008031104 nightly?) this loop runs earlier than it
used to, and the "Default browser" dialog (and presumably also the
proxy auth dialog) gets displayed in it (also earlier than they used
to).

This loop is no longer needed.  And if we get rid of it the "Default
browser" dialog (at least) goes back to being displayed on top of the
first browser window.

I haven't yet tested with the proxy auth dialog -- others will need to
do this for me.

Here's a tryserver build made with this patch:

https://build.mozilla.org/tryserver-builds/2008-04-22_12:28-smichaud@pobox.com-bugzilla424266/smichaud@pobox.com-bugzilla424266-firefox-try-mac.dmg
Assignee: enndeakin → smichaud
Status: NEW → ASSIGNED
Assignee: smichaud → joshmoz
Status: ASSIGNED → NEW
Component: Startup and Profile System → Widget: Cocoa
Flags: blocking-firefox3+
Product: Firefox → Core
QA Contact: startup → cocoa
Target Milestone: Firefox 3 → ---
Assignee: joshmoz → smichaud
Flags: blocking1.9?
Attachment #317074 - Flags: review?(joshmoz)
Mark, since it's your old workaround that I'm removing, you should probably also take a look at my patch (attachment 317074 [details] [diff] [review]).
Attachment #317074 - Flags: review?(joshmoz) → review+
Target Milestone: --- → mozilla1.9
Flags: blocking1.9? → blocking1.9+
Attachment #317074 - Flags: superreview?(roc)
Attachment #317074 - Flags: review?(mark)
Comment on attachment 317074 [details] [diff] [review]
Quick fix

I like removing this code very, very much!
Attachment #317074 - Flags: superreview?(roc) → superreview+
Comment on attachment 317074 [details] [diff] [review]
Quick fix

We should go ahead and land this, Mark can add review post-landing.
Attachment #317074 - Flags: approval1.9?
Comment on attachment 317074 [details] [diff] [review]
Quick fix

a1.9+=damons
Attachment #317074 - Flags: approval1.9? → approval1.9+
Landed on trunk:

Checking in widget/src/cocoa/nsMenuItemIconX.h;
/cvsroot/mozilla/widget/src/cocoa/nsMenuItemIconX.h,v  <--  nsMenuItemIconX.h
new revision: 1.3; previous revision: 1.2
done
Checking in widget/src/cocoa/nsMenuItemIconX.mm;
/cvsroot/mozilla/widget/src/cocoa/nsMenuItemIconX.mm,v  <--  nsMenuItemIconX.mm
new revision: 1.8; previous revision: 1.7
done

This patch still hasn't been tested with the proxy auth dialog.  Please report your results here.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
If the proxy auth dialog problem isn't fixed, it should go into a new bug.

But I _would_ like to hear about it here first -- one way or the other.
(In reply to comment #44)
> Someone should re-check bug 356742, too.
> 

The issue is still there (the sheet disappears on Tiger).
The patch was landed today.  Test again with tomorrow's Seamonkey nightly.
(In reply to comment #46)
> The patch was landed today.  Test again with tomorrow's Seamonkey nightly.
> 

I should probably have said that I checked with the 2008042312 cb-xserve02 Tinderbox build.
> I should probably have said that I checked with the 2008042312 cb-xserve02
> Tinderbox build.

Too bad :-(  Bug 356742 will need to be dealt with separately, then.
Duplicate of this bug: 431662
Attachment #317074 - Flags: review?(mark)
You need to log in before you can comment on or make changes to this bug.