Closed Bug 1311145 Opened 8 years ago Closed 6 years ago

Remove about in about:addons

Categories

(Toolkit :: Add-ons Manager, defect, P5)

defect

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox61 --- verified

People

(Reporter: andy+bugzilla, Assigned: jkt)

References

(Depends on 2 open bugs)

Details

(Whiteboard: [triaged])

Attachments

(3 files)

If you go to about:addons click on an add-on and then right click on the add-on you'll get a context menu. One of those choices is "about" which opens a modal XUL dialog.

There's little to no point in this dialog, there are many other ways to provide more information to the user, including just stuff on the add-on page.

Let's remove this dialog and the bugs that come with having to support it.
Assignee: nobody → amckay
Priority: -- → P5
Whiteboard: [triaged]
It's the almost the same as bug 571477; see bug 571477 comment 4, by the way.
Thanks, as per comments in bug 1139182 comment 35, we've got lots of other ways for add-on developers to surface this information already and can make our project that little bit simpler by removing another bit of XUL.
Please don't remove the about dialog capability. Credits are not a trivial matter in projects with lots of people. (I've got 70 translator credits in Flagfox in an overly-fancy scrollbox) This dialog is also used to credit use of copyrighted works, which might require attribution in the addon.

This has been a standard feature in the Addon Manager since inception, and is not in any way a problem. One menu item to load up a single (addon specified) XUL dialog is not a heavy maintenance burden. I don't see how the about dialog could be a prime target for simplification.
See Also: → 571477
I'm not planning on working on this, instead we'll remove this when we plan to rework the about:addons away from XUL completely. If anyone wants to do it in the meantime, please go for it with the understanding that we'll rip all this out anyway.
Assignee: amckay → nobody
One of the OpenLink's mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1374741#c72 is from this modal, I had no idea it even existed and rightly so this and the 8 year old dupe should just be fixed.

I have a preliminary patch which I will push but I need to test it when my machine stops melting.
Assignee: nobody → jkt
Attachment #8953099 - Flags: review?(mixedpuppy) → review?(aswan)
Markus, any objections to removing this?
Flags: needinfo?(mjaritz)
Attached image Example modal
For reference this is a screenshot of the modal on Linux, it largely looks broken and as Andy mentioned we have other avenues that are updated and modern to get all this info. (for example simply clicking on the addon name instead)
(In reply to Andrew Swan [:aswan] from comment #8)
> Markus, any objections to removing this?

No objections at all. Happy to hear this is being removed.

If this dialog contains any information that is not available in the details view of the extension, we should consider preserving that infromation.
Flags: needinfo?(mjaritz)
(In reply to Markus Jaritz [:designakt] (UX) from comment #11)
> If this dialog contains any information that is not available in the details
> view of the extension, we should consider preserving that infromation.

It looks like there are some fields (translators and contributors) that only appear in the About dialog, but I'd like to get us just linking to AMO instead of displaying that metadata in the browser.  Jorge is working on a PRD for that project independent of this bug.
Comment on attachment 8953099 [details]
Bug 1311145 - Removal of about extension modal in about:addons

https://reviewboard.mozilla.org/r/222384/#review228330

Thanks for doing this!
There are a few more strings that can be removed too:
https://searchfox.org/mozilla-central/rev/9a8d3f8191b15cdca89a7ce044c7bea2dd0462dc/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties#5-11
I think there's a test in browser/base/content/test/general or something that will fail if we leave those strings orphaned...
Attachment #8953099 - Flags: review?(aswan) → review+
(In reply to Andrew Swan [:aswan] from comment #13)
> It looks like there are some fields (translators and contributors) that only
> appear in the About dialog, but I'd like to get us just linking to AMO
> instead of displaying that metadata in the browser. Jorge is working on a
> PRD for that project independent of this bug.

Sounds good. Only we currently do not provide a link to AMO from about:addons.
(In reply to Markus Jaritz [:designakt] (UX) from comment #17)
> (In reply to Andrew Swan [:aswan] from comment #13)
> > It looks like there are some fields (translators and contributors) that only
> > appear in the About dialog, but I'd like to get us just linking to AMO
> > instead of displaying that metadata in the browser. Jorge is working on a
> > PRD for that project independent of this bug.
> 
> Sounds good. Only we currently do not provide a link to AMO from
> about:addons.

Yeah, I didn't express that clearly -- I'd like us to start doing linking to AMO and remove AMO metadata handling from the browser altogether.  I think we're in agreement that not having translators etc readily available in the mean time is fine (though its debatable how readily available it is right now, the About page is not all that easy to find)
Shall I file a follow up to discuss adding a AMO link? I don't think this should block this work but could be done in the same release pretty easily.

I think I addressed your comments, assuming it passes try can I merge?
Flags: needinfo?(aswan)
(In reply to Jonathan Kingston [:jkt] from comment #20)
> Shall I file a follow up to discuss adding a AMO link? I don't think this
> should block this work but could be done in the same release pretty easily.
> 
> I think I addressed your comments, assuming it passes try can I merge?

No, that project (removing AMO metadata) is already underway, but still in the planning stages.
Its good to go but I'd make sure you get a green try run first, these sorts of patches tend to break tests you would never anticipate...
Flags: needinfo?(aswan)
Jonathan, it looks like an unrelated test was referencing about.xul :(
Do you have time to fix that test and land this patch or should somebody else pick it up?
Flags: needinfo?(jkt)
I'm working on this again after my PTO and the code freeze. There are a few tests failing I think but I will check it over and assign it back if I can't get it done this week.
Flags: needinfo?(jkt)
Comment on attachment 8953099 [details]
Bug 1311145 - Removal of about extension modal in about:addons

Hey so I had to add the following to toolkit/mozapps/extensions/content/extensions.js:

// If the previous node is the discover panel which has since been disabled set to default
if (this.node.value == "addons://discover/" && !isDiscoverEnabled()) {
  this.node.value = gViewDefault;
}

This appears to be that removing the addon code causes the race condition in the test. Also I changed the file that is loaded in the test.

Could I get an feedback+ to renew your review before I merge?

Thanks
Attachment #8953099 - Flags: feedback?(aswan)
Comment on attachment 8953099 [details]
Bug 1311145 - Removal of about extension modal in about:addons

That's okay by me, it seems like we can probably just remove that whole test case from browser_discovery.js but you don't need to deal with that here.
Attachment #8953099 - Flags: feedback?(aswan) → feedback+
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3a8afc1650db
Removal of about extension modal in about:addons r=aswan
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91492c92276b
Removal of about extension modal in about:addons r=aswan
https://hg.mozilla.org/mozilla-central/rev/91492c92276b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Flags: needinfo?(jkt)
Attached image Bug1311145.gif
I can reproduce the issue on Firefox 59.0.2 (20180323154952) under Win 7 64-bit and Mac OS X 10.13.2.

The “About” option is visible in the context menu when you right click on the add-on.

This issue is verified as fixed on Firefox 61.0a1 (20180415220108) under Win 7 64-bit and Mac OS X 10.13.2.

Please see the attached video.
Status: RESOLVED → VERIFIED
Depends on: 1489752
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: