Closed
Bug 1311145
Opened 8 years ago
Closed 6 years ago
Remove about in about:addons
Categories
(Toolkit :: Add-ons Manager, defect, P5)
Toolkit
Add-ons Manager
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.
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → amckay
Priority: -- → P5
Whiteboard: [triaged]
Reporter | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fdb1252a4c7
Comment 2•8 years ago
|
||
It's the almost the same as bug 571477; see bug 571477 comment 4, by the way.
Reporter | ||
Comment 3•8 years ago
|
||
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.
Comment 4•8 years ago
|
||
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.
Reporter | ||
Comment 5•7 years ago
|
||
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
Assignee | ||
Comment 6•6 years ago
|
||
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
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Attachment #8953099 -
Flags: review?(mixedpuppy) → review?(aswan)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•6 years ago
|
||
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)
Comment 11•6 years ago
|
||
(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)
Comment 13•6 years ago
|
||
(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 14•6 years ago
|
||
mozreview-review |
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+
Comment 15•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8953099 [details] Bug 1311145 - Removal of about extension modal in about:addons https://reviewboard.mozilla.org/r/222384/#review228330 Oh and this needs to go too: https://searchfox.org/mozilla-central/rev/9a8d3f8191b15cdca89a7ce044c7bea2dd0462dc/toolkit/mozapps/extensions/test/browser/browser.ini#5
Comment hidden (mozreview-request) |
Comment 17•6 years ago
|
||
(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.
Comment 18•6 years ago
|
||
(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)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 20•6 years ago
|
||
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)
Comment 21•6 years ago
|
||
(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)
Comment 22•6 years ago
|
||
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)
Assignee | ||
Comment 23•6 years ago
|
||
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 hidden (mozreview-request) |
Assignee | ||
Comment 25•6 years ago
|
||
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 26•6 years ago
|
||
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+
Comment 27•6 years ago
|
||
Pushed by jkingston@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3a8afc1650db Removal of about extension modal in about:addons r=aswan
Comment 28•6 years ago
|
||
Backed out changeset for mochitest browser chrome failures on browser_all_files_referenced.js Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=1d17a0dffc66f757144b102448ac54e8a3036048&fromchange=3a8afc1650db3c1be5b145a7f247798d65d43338&tochange=5adc5494450ba08d4b3ae6cc39b8ee3dc3f16acb Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=170728550&repo=autoland&lineNumber=2651 Backout link: https://hg.mozilla.org/integration/autoland/rev/5adc5494450ba08d4b3ae6cc39b8ee3dc3f16acb
Flags: needinfo?(jkt)
Comment hidden (mozreview-request) |
Comment 30•6 years ago
|
||
Pushed by jkingston@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/91492c92276b Removal of about extension modal in about:addons r=aswan
Comment 31•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/91492c92276b
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jkt)
Comment 32•6 years ago
|
||
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
Updated•6 years ago
|
status-firefox52:
affected → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•