Show bookmarks/history should make a new window when window is minimized

RESOLVED FIXED in Camino1.5

Status

RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: froodian, Assigned: froodian)

Tracking

({fixed1.8.1})

unspecified
Camino1.5
PowerPC
macOS
fixed1.8.1

Details

Attachments

(1 attachment, 2 obsolete attachments)

1.60 KB, patch
stuart.morgan+bugzilla
: review+
mikepinkerton
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

13 years ago
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).
(Assignee)

Comment 1

13 years ago
Posted patch Patch (obsolete) — Splinter Review
This does in fact fix the other bug too, btw.
Assignee: nobody → stridey
Status: NEW → ASSIGNED
Attachment #226148 - Flags: review?(bugzilla)
Comment on attachment 226148 [details] [diff] [review]
Patch

r=me. Looks good.
Attachment #226148 - Flags: review?(bugzilla) → review+
(Assignee)

Updated

13 years ago
Attachment #226148 - Flags: review?(stuart.morgan)

Comment 3

13 years ago
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.
Attachment #226148 - Flags: review?(stuart.morgan) → review-
(Assignee)

Comment 4

13 years ago
Attachment #226148 - Attachment is obsolete: true
Attachment #227193 - Flags: review?(stuart.morgan)

Comment 5

13 years ago
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.
Attachment #227193 - Flags: superreview?(mikepinkerton)
Attachment #227193 - Flags: review?(stuart.morgan)
Attachment #227193 - Flags: review+
(Assignee)

Comment 6

13 years ago
Posted patch Uses helper method (obsolete) — Splinter Review
While this is just sitting here, I might as well do it the nice way.
Attachment #227193 - Attachment is obsolete: true
Attachment #231334 - Flags: superreview?(mikepinkerton)
Attachment #231334 - Flags: review?(stuart.morgan)
Attachment #227193 - Flags: superreview?(mikepinkerton)

Comment 7

13 years ago
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.
Attachment #231334 - Flags: superreview?(mikepinkerton)
Attachment #231334 - Flags: review?(stuart.morgan)
Attachment #231334 - Flags: review-

Updated

13 years ago
Attachment #227193 - Attachment is obsolete: false
Attachment #227193 - Flags: superreview?(mikepinkerton)
Comment on attachment 227193 [details] [diff] [review]
Follows behavior elsewhere

sr=pink.
Attachment #227193 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Updated

13 years ago
Attachment #231334 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Whiteboard: [needs checkin]

Comment 9

13 years ago
Fixed trunk and branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [needs checkin]
You need to log in before you can comment on or make changes to this bug.