When the about window is opened the app menu's check for updates menuitem is a noop

VERIFIED FIXED in Firefox 4.0b8

Status

()

Firefox
Menus
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: rstrong, Assigned: Margaret)

Tracking

({polish, user-doc-needed})

Trunk
Firefox 4.0b8
All
Mac OS X
polish, user-doc-needed
Points:
---
Dependency tree / graph
Bug Flags:
in-litmus +

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Bug 596813 made it so the about window contains a minimal version of the app update ui. On Mac OS X the app menu still contains a check for updates which attempts to open the app update ui but I intentionally added checks in the app update service to prevent this from happening since the two ui's would fight over being listeners for different actions, etc.

So, the app menu's check for updates menuitem should either be removed or focus the about window when it is opened.
This should block IMO
blocking2.0: --- → ?
Doesn't this conflict with bug 306018 that was fixed recently?  I think they didn't want OSX to use the about dialog per bug 585475 comments 11-13.
It doesn't conflict if UX choose to just focus the about window when it is opened. As for standard locations, the location that is the most standard for apps to provide a check for updates on Windows is under Help so I don't see why it can't be decided by UX for it to different on Mac as it was decided that it should be different on Windows.

Another option would be to disable app update in the about window on Mac OS X.
Should focus the About Window, IMO.
blocking2.0: ? → final+
Keywords: polish
(Assignee)

Comment 5

8 years ago
Created attachment 484031 [details] [diff] [review]
patch

This patch opens the about dialog instead of the update dialog. It seems to do the right thing, but I haven't tested it with a test update available.
Assignee: nobody → margaret.leibovic
Attachment #484031 - Flags: feedback?(robert.bugzilla)
Comment on attachment 484031 [details] [diff] [review]
patch

Should be fine with this approach
Attachment #484031 - Flags: feedback?(robert.bugzilla) → feedback+
(Assignee)

Updated

8 years ago
Attachment #484031 - Attachment description: WIP → patch
Attachment #484031 - Flags: review?(dolske)
(Assignee)

Updated

8 years ago
Whiteboard: [needs review dolske]
Hmm, I've got a general concern about doing this... When the user explicitly selects Check For Update, it's a little confusing to have a giant, wordy About dialog pop up -- of which the update related portion is a small and subtle part. Doubly so because the "About Firefox" and "Check For Update" menuitems are right next to each other, so people might think they misclicked.

Having a separate dialog seems better in that respect. We could just make Check For Updates focus the About window only in the rare event that it's already open (and keep the existing behavior otherwise). I suppose opening the About window would similarly need to raise the update check window if it happens to have already been opened?

If that's too hacky, it might be OK to go with the current patch if we also improve the About window to make the status of updates more obvious. Maybe that's a good idea anyway. Specifically:

  * Keep the checking message+throbber for a minimum of 2 seconds, so the user has a chance to see it's actually doing something (ie, it draws their attention). I've noticed on fast connections the check completes so fast I'm not sure if it's just caching the status or actually doing something.

  * Provide some context (colored?) for the "Apply Update" when present. EG "A newer version is available! [Apply Update]".
I personally don't see why it is ok to remove Check for Updates... from the help menu on Windows and it isn't ok to remove it from the app menu on Mac OS X.
Yeah, per discussion, I think we should just remove the Check For Updates menu on OS X too. Sad to do now that we've finally got it in the right place, but meh. Also, note that neither Chrome nor Safari have a Check For Updates menuitem there.
(In reply to comment #7)
> Hmm, I've got a general concern about doing this... When the user explicitly
> selects Check For Update, it's a little confusing to have a giant, wordy About
> dialog pop up -- of which the update related portion is a small and subtle
> part. Doubly so because the "About Firefox" and "Check For Update" menuitems
> are right next to each other, so people might think they misclicked.

That's a good point, and one that I hadn't considered. Likely also the reason why they took it out of Chromium for OSX. The other reason, I suspect, is because they just expect Chrome to automatically update.

Meh, I guess I'm fine with getting rid of the menuItem. We'll have to update all our docs to say "Go to About Firefox" to check for updates. w/e.
Keywords: user-doc-needed
Comment on attachment 484031 [details] [diff] [review]
patch

Clearing review, needs patch that just removes the menuitem per last comment.
Attachment #484031 - Flags: review?(dolske)
Whiteboard: [needs review dolske]
(Assignee)

Comment 12

8 years ago
Created attachment 485795 [details] [diff] [review]
patch v2

This patch gets rid of the check for updates menuitem. I reverted the changes from bug 306018. I also had to change some references to the updateSeparator element, since I got rid of that.
Attachment #484031 - Attachment is obsolete: true
Attachment #485795 - Flags: review?(dolske)
We should probably just leave the nsMenuBarX code in for other apps that may want to keep the menu item.
(Assignee)

Comment 14

8 years ago
Created attachment 485869 [details] [diff] [review]
patch v3

New patch that keeps the widget code.
Attachment #485795 - Attachment is obsolete: true
Attachment #485869 - Flags: review?(dolske)
Attachment #485795 - Flags: review?(dolske)
(Assignee)

Updated

8 years ago
Whiteboard: [needs review dolske]
(Assignee)

Updated

8 years ago
Duplicate of this bug: 599945
Blocks: 599290
Comment on attachment 485869 [details] [diff] [review]
patch v3

Not sure if there are any tests exercising this code, might want to just run this through try unless you've already checked.
Attachment #485869 - Flags: review?(dolske) → review+
(Assignee)

Updated

8 years ago
Whiteboard: [needs review dolske] → [can land]
(Assignee)

Comment 17

8 years ago
http://hg.mozilla.org/mozilla-central/rev/726e585dd529
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101106 Firefox/4.0b8pre

Updated the Litmus test:
https://litmus.mozilla.org/show_test.cgi?id=9940
Status: RESOLVED → VERIFIED
Flags: in-litmus+
Target Milestone: --- → Firefox 4.0b8

Updated

8 years ago
Depends on: 306018

Updated

8 years ago
Depends on: 616146
Depends on: 621805
Depends on: 615513
Depends on: 666086
You need to log in before you can comment on or make changes to this bug.