The default bug view has changed. See this FAQ.

Disable menus commands when sheets or dialogs are frontmost

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
Toolbars & Menus
P3
minor
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: Stephane Moureau, Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.2})

unspecified
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.2

Details

Attachments

(1 attachment, 2 obsolete attachments)

14.15 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.0.1) Gecko/20021122 Chimera/0.6+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.0.1) Gecko/20021122 Chimera/0.6+

"Sheet Behavior
...
Only one sheet may be open for a window at any one time. A sheet PREVENTS ANY
OTHER OPERATION on that window until the sheet is dismissed. If, when the user
responds to a sheet, another sheet for that document must open, the first sheet
closes before the second one opens."

Reproducible: Always

Steps to Reproduce:
Open a New Window with the Toolbar and Location field.
Reduce its size to the minimum with the Grow icon.
Click on the double arrow in the Toolbar and select Location.
Min/Max-imizes, Resize, Save, Print, Find, Go, Bookmarks commands are available.

Comment 1

15 years ago
confirmed in 2002111604. Severity to trivial (it was enhancement)... There's no
harm done by this bug AFAICT, although if/when sheets are used for printing
there might be more of an issue.
Severity: enhancement → trivial
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

14 years ago
Summary: AAHIG: Page Sheet allows other operations → Disable commands when OpenLocation sheet is up
hahah cute.
Assignee: saari → pinkerton
Target Milestone: --- → Camino1.0
Blocks: 294129
This is still present and should definitely be fixed by 1.0. :)
Target Milestone: Camino1.0 → Camino1.1

Comment 4

11 years ago
I don't see any harm in this; I wasn't able to get into a locked state with the fix for bug 314072 in my tree.
For polish, we should probably grey out some additional menu items that currently just beep (Open File´┐Ż), but I can't get into a locked state anywhere, either.

Updated

11 years ago
Severity: trivial → minor
Priority: -- → P3
Summary: Disable commands when OpenLocation sheet is up → Disable menus commands when sheets or dialogs are frontmost

Comment 6

11 years ago
*** Bug 187849 has been marked as a duplicate of this bug. ***

Comment 7

11 years ago
I disabled openFile: and openLocation: when the frontmost window has a sheet up. Loading bookmarks seems harmless, and as we don't block all the other various ways you can change the browser content, I don't see a compelling reason to disable bookmarks.

Comment 8

11 years ago
Really? You can load bookmarks and change the browser content when a sheet is open on that page? Doesn't that fall under the "ANY OTHER OPERATION" restriction?

Comment 9

11 years ago
(In reply to comment #8)
> Really? You can load bookmarks and change the browser content when a sheet is
> open on that page? Doesn't that fall under the "ANY OTHER OPERATION"
> restriction?

Yes, but rather than disable all the commands that can possibly change the window ocontent, and risk introducing bugs at this stage in the release (close to 1.0), I just fixed the most obvious issue. That's why this bug is still open.
*** Bug 329331 has been marked as a duplicate of this bug. ***
Component: General → Toolbars & Menus
QA Contact: winnie → toolbars
*** Bug 333438 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Blocks: 341853

Comment 12

11 years ago
Taking.
Assignee: mikepinkerton → bugzilla
Whiteboard: [good first bug]
*** Bug 347630 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 14

11 years ago
Taking; I'll fix this after bug 159337 lands.
Assignee: bugzilla → stuart.morgan
(Assignee)

Comment 15

11 years ago
Created attachment 245736 [details] [diff] [review]
fix

This turned into sort of a game of whack-a-mole once I realized that all the context menus needed disabling too. This should cover everything with two exceptions:
- Use as Dock Menu, which works and is harmless, and therefore doesn't seem worth adding a menu validator to the bookmark class or a passthrough in some other class for.
- Show/Hide Toolbar, which Cocoa should be handling for us, and I can't figure out how we're confusing it. It doesn't do any harm, so we can file it as a low-priority follow-up bug.
Attachment #245736 - Flags: review?
(Assignee)

Comment 16

11 years ago
Created attachment 245743 [details] [diff] [review]
full fix

Missed a file.
Attachment #245736 - Attachment is obsolete: true
Attachment #245743 - Flags: review?
Attachment #245736 - Flags: review?

Comment 17

11 years ago
Comment on attachment 245743 [details] [diff] [review]
full fix

>@@ -1486,19 +1498,34 @@
> 
>   // >NSLog(@"MainController validateMenuItem for %@ (%s)", [aMenuItem title], action);
> 
>+  // disable window-related menu items if a sheet is up
>+  if (browserController && [[browserController window] attachedSheet] &&

Patch looks good, except it fails to account for sheets on non-browser windows.

For example, on the Downloads window, you can Customise Toolbar to get a sheet, and observe that nothing gets disabled.

r=me with that issue addressed.
Attachment #245743 - Flags: review? → review-
(Assignee)

Comment 18

11 years ago
(In reply to comment #17)
> Patch looks good, except it fails to account for sheets on non-browser windows.
> 
> For example, on the Downloads window, you can Customise Toolbar to get a sheet,
> and observe that nothing gets disabled.

Erm... what exactly are you looking for to be disabled when there's a sheet on the downloads window? None of the loading sorts of things apply to it.

Comment 19

11 years ago
(In reply to comment #18)
> Erm... what exactly are you looking for to be disabled when there's a sheet on
> the downloads window? None of the loading sorts of things apply to it.

Yeah, upon looking over the File menu again, I guess none of that stuff really needs to be disabled anyway (although I still think it's weird that stuff like "Search the Web" and "Open Location..." are active when a non-browser window is frontmost).

cl
(Assignee)

Comment 20

11 years ago
So is there still a reason this is minused?

Comment 21

11 years ago
Comment on attachment 245743 [details] [diff] [review]
full fix

No. :)
Attachment #245743 - Flags: review- → review+
(Assignee)

Updated

11 years ago
Attachment #245743 - Flags: superreview?(mikepinkerton)
(In reply to comment #19)
> (although I still think it's weird that stuff like
> "Search the Web" and "Open Location..." are active when a non-browser window is
> frontmost).

So you can get focus back to the browser window :P

Updated

11 years ago
No longer blocks: 294129
Comment on attachment 245743 [details] [diff] [review]
full fix

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

Comment 24

10 years ago
Created attachment 249776 [details] [diff] [review]
unbitrotted

As checked in on trunk and MOZILLA_1_8_BRANCH.
Attachment #245743 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: fixed1.8.1.2
Resolution: --- → FIXED
Whiteboard: [good first bug]
You need to log in before you can comment on or make changes to this bug.