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: --- → ?
8 years ago
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+
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+
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.
Comment on attachment 484031 [details] [diff] [review] patch Clearing review, needs patch that just removes the menuitem per last comment.
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.
We should probably just leave the nsMenuBarX code in for other apps that may want to keep the menu item.
Created attachment 485869 [details] [diff] [review] patch v3 New patch that keeps the widget code.
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+
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
Target Milestone: --- → Firefox 4.0b8
You need to log in before you can comment on or make changes to this bug.