Closed Bug 590127 Opened 9 years ago Closed 4 years ago
Remove toolkit implementation of about:addons
about:addons UI doesn't work well at all in fennec, so it should be removed since it creates a bad user experience. This page is listed in about:about
This about: page is added in the toolkit AboutRedirector component : http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsAboutRedirector.cpp?force=1
Should it be removed or fixed by making fields more easily accessible? I mean, it's not too bad on my Linux Fennec, just some scrolling is needed and sometimes buttons are too close to right edge of the screen.
Fennec doesn't use about:addons, and this bug is just about removing it so users don't stumble upon it and try to use it.
I started working on this
I wonder if this patch is enough to stop access to about:addons. This is effective only on Maemo. Should this be blocked also from other OS?
Marko, We need this to be disabled for all mobile platforms, not just Maemo. The env var MOZ_APP_NAME is set to "mobile" in such cases
Another thing to consider is trying to remove the actual code shipped with the toolkit to support about:addons We might be able to reduce our installed footprint if we can stop shipping code we don't use.
Just to be sure, should about:addons be removed totally? (not only for mobiles)
(In reply to comment #9) > Just to be sure, should about:addons be removed totally? (not only for mobiles) No, only for mobile
Litmus test created https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=12965 Needs update once this bug is fixed.
Flags: in-litmus?(nhirata.bugzilla) → in-litmus+
(In reply to comment #7) > We need this to be disabled for all mobile platforms, not just Maemo. The env > var MOZ_APP_NAME is set to "mobile" in such cases Should we create such precompiler macro/definition indicating mobile platform, because environmental variable is not checked on compilation phase?
This patch is checking that none of the following is defined: ANDROID MOZ_PLATFORM_MAEMO SYMBIAN WINCE_WINDOWS_MOBILE What comes to removing the actual contents of chrome://mozapps/content/extensions/extensions.xul, I guess it can't be done because it's not on mobile branch.
Marko, extensions.xul is included because it is referenced in toolkit/mozapps/extensions/jar.mn You can use ifdefs here also (e.g. http://mxr.mozilla.org/mozilla-central/source/toolkit/components/printing/jar.mn#4) The last one to remove is toolkit/mozapps/extensions/content/extensions.js, included from the same jar.mn (also, can you mark your previous patches as obsolete?)
(In reply to comment #13) > Created attachment 477865 [details] [diff] [review] > Patch to prevent opening about:addons from mobile OS > > This patch is checking that none of the following is defined: > ANDROID > MOZ_PLATFORM_MAEMO > SYMBIAN > WINCE_WINDOWS_MOBILE You can use MOZ_GFX_OPTIMIZE_MOBILE instead of all of those
Now using MOZ_GFX_OPTIMIZE_MOBILE
Attachment #477865 - Attachment is obsolete: true
can we just load fennec mobile extensions page which is kind of usable and provide good user experience ?
(In reply to comment #16) > Created attachment 478194 [details] [diff] [review] > Patch to prevent opening about:addons from mobile OS (v4) > > Now using MOZ_GFX_OPTIMIZE_MOBILE When Windows/Mac/Linux fennec build (not Maemo and Android), we need --enable-mobile-optimize to use this define. As long as I check build config for win32, this config flag isn't tuned on. (http://hg.mozilla.org/build/buildbot-configs/file/83c96267d655/mozilla2/mobile/win32-i686/mobile-browser/nightly/mozconfig) Also, we shouldn't include desktop addon manager xul.
MOZ_GFX_OPTIMIZE_MOBILE is the wrong #define to be using.
Here is a patch to exclude about:addons from mobile builds. I modified configure.in to introduce a define that looks like MOZ_BUILD_APP_MOBILE. We already have this info in MOZ_BUILD_APP but that cannot be used in source code since the preprocessor cannot compare strings.
Assignee: nobody → sarentz
Attachment #478194 - Attachment is obsolete: true
Better patch with a fix for configure.in where MOZ_BUILD_APP_standalone had the wrong case.
Attachment #536721 - Attachment is obsolete: true
Comment on attachment 536735 [details] [diff] [review] Patch to exclude about:addons from mobile builds Review of attachment 536735 [details] [diff] [review]: -----------------------------------------------------------------
Comment on attachment 536735 [details] [diff] [review] Patch to exclude about:addons from mobile builds I don't think we want to add general purpose MOZ_BUILD_APP_* defines. There should probably be a global "mobile" define (or perhaps just rename MOZ_GFX_OPTIMIZE_MOBILE?), but it should be independent of MOZ_BUILD_APP, and maybe just be set in mobile's confvars.sh.
Attachment #536735 - Flags: feedback-
Ok. How about simply setting MOZ_MOBILE from mobile/confvars.sh?
Product: Fennec Graveyard → Firefox for Android
Summary: remove about:addons → Remove toolkit implementation of about:addons
Updated to Native Fennec. We have our own about:addons, and just need to not include the toolkit version in Fennec.
Assignee: nobody → mark.finkle
Attachment #536735 - Attachment is obsolete: true
Try found a test that hardcodes xpinstallConfirm.js, so I just skipped that part for now: http://mxr.mozilla.org/mozilla-central/source/caps/tests/mochitest/test_bug292789.html?force=1#16 Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=20c27fb81ad6
(In reply to Mark Finkle (:mfinkle) from comment #28) > Try: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=20c27fb81ad6 Try looks good. We might get ~110KB reduction in APK size from this change.
Comment on attachment 8703739 [details] [diff] [review] remove-extensionsui v0.2 Review of attachment 8703739 [details] [diff] [review]: ----------------------------------------------------------------- I don't think we have much automated test coverage for add-ons UI interactions, so we should keep an eye out for regressions. ::: toolkit/mozapps/extensions/jar.mn @@ +30,5 @@ > content/mozapps/extensions/newaddon.js (content/newaddon.js) > content/mozapps/extensions/setting.xml (content/setting.xml) > content/mozapps/extensions/pluginPrefs.xul (content/pluginPrefs.xul) > content/mozapps/extensions/gmpPrefs.xul (content/gmpPrefs.xul) > content/mozapps/extensions/OpenH264-license.txt (content/OpenH264-license.txt) We do include the OpenH264 plugin on mobile... we don't need these files? Looking at the item in about:addons, I do see that we don't show any license information or add-on options, but that might be its own bug.
Attachment #8703739 - Flags: review?(margaret.leibovic) → review+
You need to log in before you can comment on or make changes to this bug.