Closed Bug 348419 Opened 15 years ago Closed 14 years ago

No way to access the update notification dialog when visiting an extension's homepage

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha7

People

(Reporter: zeniko, Assigned: robert.strong.bugs)

References

Details

(Keywords: verified1.8.1.8)

Attachments

(1 file, 2 obsolete files)

Steps to reproduce:
1. Make sure that you get an update notification on startup (e.g. lower an extension's version in extensions.rdf, find the update for it in the Add-ons manager and set extensions.update.notifyUser to true)
2. Launch Firefox
3. Visit that extension's homepage
4. Try to continue the update process...

Actual result:
You have to close the opened browser window before being able to get back (NB: people resuming their previous session will lose it in the process).

Expected result:
? (not sure how to best handle this case - maybe don't allow the homepage to be visited in this case, or don't make the notification dialog modal)
For 2.0 this should just not be available and for 3.0 I'd like an app notification area for notification of updates instead of notifying on startup which would solve this.
bug 347585 is for the app notification area 
Summary: No way to access the update notification dialog after visiting an extension's homepage → No way to access the update notification dialog when visiting an extension's homepage
Too late to try to fix this for 2.0 and for 3.0 fixing bug 347585 and making the EM use the notification area will fix this.
Depends on: 347585
Depends on: 366777
No longer depends on: 347585
Blocks: 366777
No longer depends on: 366777
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Dave, what do you think about removing the option to open the homepage and the about dialog?
I can't say I'm all that keen on it. I think it's hiding potentially crucial information from the user when he might need it most. In my experience most users barely know what add-ons the have installed so when an update reminder appears I think they might want to find out just what that thing is. I guess it depends what happens with bug 297903.

That said I'm willing to bet hardly anyone uses them in the update reminder and I can't think of a decent way to make it work in the "notify before startup" scenario.
Agreed on all counts... I'm going to run this by beltzner.
Seth, I talked with beltzner about this approach and got his buy in to make this change.
Attachment #271480 - Attachment is obsolete: true
Attachment #271940 - Flags: review?(sspitzer)
Comment on attachment 271940 [details] [diff] [review]
patch (remove Homepage and About context menuitems for Firefox only)

I think it might be better to explicitly call out what items you are removing.

Something like: 

#ifdef MOZ_PHOENIX
        // If we are a browser when updating on startup don't display context
        // menuitems that can open a browser window.
        gUpdateContextMenus.splice(gUpdateContextMenus.indexOf("menuitem_homepage"), 1);
        gUpdateContextMenus.splice(gUpdateContextMenus.indexOf("menuitem_about"), 1);
        gUpdateContextMenus.splice(gUpdateContextMenus.indexOf("menuseparator_1"), 1);
#endif
What is the concern this would address?
my reason is that so if someone changes the order of those menuitems or changes the code, they're more likely to see the code that dynamically modifies that array.

How about doing something like this, instead:

var gUpdateContextMenusNoBrowser = ["menuitem_installUpdate",
"menuitem_includeUpdate"];
var gUpdateContextMenus = ["menuitem_homepage", "menuitem_about",
"menuseparator_1"].concat(gUpdateContextMenusNoBrowser);

#ifdef MOZ_PHOENIX
  // If we are a browser when updating on startup don't display context
  // menuitems that can open a browser window.
  gUpdateContextMenus = gUpdateContextMenusNoBrowser;
#endif
I like the second approach better since they may also be removed instead of just changed. I'll go with that and resubmit. Thanks
Comment on attachment 272073 [details] [diff] [review]
patch rev 3 (remove Homepage and About context menuitems for Firefox only)

r=sspitzer, thanks robert
Attachment #272073 - Flags: review?(sspitzer) → review+
Checked in to trunk

Checking in mozilla/toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <--  extensions.js
new revision: 1.125; previous revision: 1.124
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Duplicate of this bug: 361129
Target Milestone: --- → Firefox 3 M7
Comment on attachment 272073 [details] [diff] [review]
patch rev 3 (remove Homepage and About context menuitems for Firefox only)

This patch fixes this bug by preventing users from opening a browser window on firefox during startup which over-writes the session store as seen in Bug 361129.
Attachment #272073 - Flags: approval1.8.1.6?
Now that SeaMonkey is using toolkit can the phrase in the comment "If we are a browser" be changed to "If we are Firefox"?
Comment on attachment 272073 [details] [diff] [review]
patch rev 3 (remove Homepage and About context menuitems for Firefox only)

approved for 1.8.1.7, a=dveditz for release-drivers
Attachment #272073 - Flags: approval1.8.1.7? → approval1.8.1.7+
Checked in to MOZILLA_1_8_BRANCH

Checking in mozilla/toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <--  extensions.js
new revision: 1.72.2.43; previous revision: 1.72.2.42
done
Keywords: fixed1.8.1.7
verified fixed 1.8.1.8 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.8pre) Gecko/20070929 BonEcho/2.0.0.8pre ID:2007092904 - the context menu for visit homepage and about is removed, so no new browser could be opened when you see the update notification - comment #17

- > adding verified keyword

Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.