Last Comment Bug 341897 - Show bookmarks/history should make a new window when window is minimized
: Show bookmarks/history should make a new window when window is minimized
Status: RESOLVED FIXED
: fixed1.8.1
Product: Camino Graveyard
Classification: Graveyard
Component: Toolbars & Menus (show other bugs)
: unspecified
: PowerPC Mac OS X
-- normal (vote)
: Camino1.5
Assigned To: froodian (Ian Leue)
:
:
Mentors:
Depends on:
Blocks: 324486
  Show dependency treegraph
 
Reported: 2006-06-17 18:37 PDT by froodian (Ian Leue)
Modified: 2006-08-02 15:42 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (909 bytes, patch)
2006-06-19 07:22 PDT, froodian (Ian Leue)
bugzilla-graveyard: review+
stuart.morgan+bugzilla: review-
Details | Diff | Splinter Review
Follows behavior elsewhere (1.60 KB, patch)
2006-06-26 23:04 PDT, froodian (Ian Leue)
stuart.morgan+bugzilla: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review
Uses helper method (6.60 KB, patch)
2006-07-30 13:46 PDT, froodian (Ian Leue)
stuart.morgan+bugzilla: review-
Details | Diff | Splinter Review

Description User image froodian (Ian Leue) 2006-06-17 18:37:42 PDT
STR:
1. Open a single window
2. Minimize
3. Cmd-B or Cmd-Y

What happens: Collection is opened in minimized window.

Bug 324486 Comment 2: "The fact that we don't disable the "Show [collection]" menu items probably ought to be filed separately."

Per IRC, rather than disable the menu items, we should do what Cmd-T does in this situation, which is to create a new window containing the appropriate collection.

I bet this'll fix bug 324486 (or rather, make it impossible to trigger).
Comment 1 User image froodian (Ian Leue) 2006-06-19 07:22:18 PDT
Created attachment 226148 [details] [diff] [review]
Patch

This does in fact fix the other bug too, btw.
Comment 2 User image Chris Lawson (gone) 2006-06-19 15:32:00 PDT
Comment on attachment 226148 [details] [diff] [review]
Patch

r=me. Looks good.
Comment 3 User image Stuart Morgan 2006-06-26 22:12:19 PDT
Comment on attachment 226148 [details] [diff] [review]
Patch

The behavior of other front-browser-window-affecting commands (home, search, location) is to makeKeyAndOrderFront: on the window if it needs it, not make a new window.  I think this should do the same for consistency.
Comment 4 User image froodian (Ian Leue) 2006-06-26 23:04:40 PDT
Created attachment 227193 [details] [diff] [review]
Follows behavior elsewhere
Comment 5 User image Stuart Morgan 2006-06-29 20:04:27 PDT
Comment on attachment 227193 [details] [diff] [review]
Follows behavior elsewhere

r=me, although I wonder if it might be worth making some sort of ensureFrontmostBrowserWindow  that encapsulates getting the frontmost window, making it or keying it if necessary.
Comment 6 User image froodian (Ian Leue) 2006-07-30 13:46:26 PDT
Created attachment 231334 [details] [diff] [review]
Uses helper method

While this is just sitting here, I might as well do it the nice way.
Comment 7 User image Stuart Morgan 2006-07-30 22:21:53 PDT
Comment on attachment 231334 [details] [diff] [review]
Uses helper method

This will cause some undesirable behavior changes: openLocation: would potentially open a window that is loading the homepage, doing a goHome: with no windows would double-load the homepage, and possibly other things as well.

Let's just stick with the previous version, and tackle refactoring in another bug if it's really worthwhile.
Comment 8 User image Mike Pinkerton (not reading bugmail) 2006-08-02 07:47:55 PDT
Comment on attachment 227193 [details] [diff] [review]
Follows behavior elsewhere

sr=pink.
Comment 9 User image Nick Kreeger 2006-08-02 15:42:01 PDT
Fixed trunk and branch.

Note You need to log in before you can comment on or make changes to this bug.