Closed Bug 599480 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox :: Menus, defect)

All
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b8
Tracking Status
blocking2.0 --- final+

People

(Reporter: rstrong, Assigned: Margaret)

References

Details

(Keywords: polish, user-doc-needed)

Attachments

(1 file, 2 obsolete files)

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
Attached patch patch (obsolete) — Splinter Review
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+
Attachment #484031 - Attachment description: WIP → patch
Attachment #484031 - Flags: review?(dolske)
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]
Attached patch patch v2 (obsolete) — Splinter Review
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.
Attached patch patch v3Splinter Review
New patch that keeps the widget code.
Attachment #485795 - Attachment is obsolete: true
Attachment #485869 - Flags: review?(dolske)
Attachment #485795 - Flags: review?(dolske)
Whiteboard: [needs review dolske]
Duplicate of this bug: 599945
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+
Whiteboard: [needs review dolske] → [can land]
http://hg.mozilla.org/mozilla-central/rev/726e585dd529
Status: NEW → RESOLVED
Closed: 9 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
Depends on: 306018
Depends on: 616146
You need to log in before you can comment on or make changes to this bug.