New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Menu Bar icon to be optional #84

Closed
walkintom opened this Issue Nov 9, 2018 · 22 comments

Comments

Projects
None yet
@walkintom
Copy link

walkintom commented Nov 9, 2018

Many of us like an uncluttered and clean menu bar, I feel that having an icon in Safari's menu bar is enough to know the extension is enabled, is a OS menu bar icon necessary as well? At the very least it could be optional?

UPD by @ameshkov: #84 (comment)

@ameshkov

This comment has been minimized.

Copy link
Member

ameshkov commented Nov 9, 2018

That's questionable. The problem is that now we can't have just the extension. There must be an app that controls it and removing the menu bar icon will basically mean that the app is running in background with no sign of it anywhere.

@walkintom

This comment has been minimized.

Copy link

walkintom commented Nov 9, 2018

True, but I see that as perfect justification to make it an optional feature, especially for power users who have chosen to launch the app as a login item, and already know it’ll be running in the background. BetterTouchTool is an example of an app that gives this option.

@milohuang

This comment has been minimized.

Copy link

milohuang commented Nov 11, 2018

After installing the App Store version and enabling filters, the extension works fine. Without enabling launch at login, the extension on Safari 12 works as it used to (that it, blocking ads even without AdGuard running). My guess is the filter rules were already imported in Safari's content blocker that you no longer need an extension to work the magic, but will need when adding or removing filers.

@tiiiecherle

This comment has been minimized.

Copy link

tiiiecherle commented Nov 13, 2018

That said it raises the question if the filter rules are updated without having the app open?

So to make the question clearer: Does the app have to be started to update filter lists or is it enough to enable the adguard extension in safari (and only one, not the safari icon) to update and use filter lists?

Thanks in advance for the really good work and the clarification.

@dar-irl

This comment has been minimized.

Copy link

dar-irl commented Nov 13, 2018

I'm also interested in whether or not the menu bar icon is necessary -- Wipr, for example, is able to function without one.

@Shnub

This comment has been minimized.

Copy link

Shnub commented Nov 18, 2018

I would also request an option to run the app without menu bar icon. As walkintom said, BetterTouchTool implements this nicely: when the option is enabled, the first/actual app launch (e. g. at login) will cause it to run in the background, and then "launching" it a second time (e. g. by double-clicking the app in the Finder) will open the preferences/configuration window.

Also, why not give the actual content blocker extension a proper button in Safari? If I install an extension, I expect something to show up anyway, and it could serve as a nice shortcut to the preferences window. No menu bar icon would be needed. Or is Apple prohibiting this now as well?

@mkfd

This comment has been minimized.

Copy link

mkfd commented Nov 25, 2018

just like 1Password,an icon on Safari will be enough,there is no need to have an icon on finder menu

@saagarjha

This comment has been minimized.

Copy link

saagarjha commented Dec 3, 2018

@ameshkov I'm not quite sure I understand your response:

The problem is that now we can't have just the extension. There must be an app that controls it and removing the menu bar icon will basically mean that the app is running in background with no sign of it anywhere.

Why is this required? What functionality do we need the main app to be running for that we can't perform in the Safari app extension context? The only thing I expect the extension to need to do is to control whether blocking is enabled and allow for selecting elements to block, both of which don't seem to require running the main app at all–the first can be done with native code in the toolbar popover, and the second by running JavaScript in-page. I haven't looked at the code very closely, but it really seems to me that this can be done by utilizing the shared container in a clever way to keep application state consistent. I really don't want to have to continuously run the main app, partially because it puts an icon in my menubar, and partially because it's Electron…

@Sugaroverdose

This comment has been minimized.

Copy link

Sugaroverdose commented Dec 4, 2018

That's questionable. The problem is that now we can't have just the extension. There must be an app that controls it and removing the menu bar icon will basically mean that the app is running in background with no sign of it anywhere.

But it still accessible via safari icon and by opening .app, so it just makes some ppl menubar cleaner, you may just add warning message to it if you feel that user might not understand that application will still run in background

@saagarjha

This comment has been minimized.

Copy link

saagarjha commented Dec 5, 2018

that user might not understand that application will still run in background

Again, I still don't understand this at all. Why must the app run in the background? It is not clear to me at all why this is required, rather than the app is running in the background.

@Sugaroverdose

This comment has been minimized.

Copy link

Sugaroverdose commented Dec 5, 2018

that user might not understand that application will still run in background

Again, I still don't understand this at all. Why must the app run in the background? It is not clear to me at all why this is required, rather than the app is running in the background.

I think that this issue is only about menubar icon, it's better to make separate issue if you feel that working in background is a wrong way.

@saagarjha

This comment has been minimized.

Copy link

saagarjha commented Dec 6, 2018

I mean, if we don't need the application to be running at all most of the time I couldn't really care less whether the menu bar item exists. I suspect most of the people here only care because it shows up 100% of the time, even when they're not actively interacting with the application or really have any need to use the menu bar item.

@sweetppro

This comment has been minimized.

Copy link

sweetppro commented Dec 6, 2018

I have a content blocker in the Mac App Store, and you really don't need the app running the entire time. You need the blocker extension, and some way to interact with it - either via a standalone app, or the toolbar icon. the toolbar is the best solution.

you could easily notify users of filter updates via the toolbar icon, and most likely even download and apply the updates if need be. The main app should only be used for Preferences - that's how mine works.
:)

@saagarjha

This comment has been minimized.

Copy link

saagarjha commented Dec 6, 2018

Yes, exactly; that's how I had imagined this working based on the API we are provided by SafariServices. I don't expect to open the main app more than once every couple of months. This also makes the menu bar item useless because the app will not be running in the background constantly anymore–it will only be running when it is showing a window on the screen.

@Sugaroverdose

This comment has been minimized.

Copy link

Sugaroverdose commented Dec 6, 2018

I have a content blocker in the Mac App Store, and you really don't need the app running the entire time. You need the blocker extension, and some way to interact with it - either via a standalone app, or the toolbar icon. the toolbar is the best solution.

you could easily notify users of filter updates via the toolbar icon, and most likely even download and apply the updates if need be. The main app should only be used for Preferences - that's how mine works.
:)

I think it's tricky to do such when app is actually parses rules and prepare them for safari-filter, which is one of things which adguard do - it supports custom adblock filter lists

@sweetppro

This comment has been minimized.

Copy link

sweetppro commented Dec 6, 2018

if its resource intensive, then you could just notify of filter updates in the toolbar icon, and prompt to open the main app to process the updated lists.

pausing AdGuard would be pretty trivial to do from the toolbar. my extension does this no problem

@ameshkov ameshkov added this to the 1.2 milestone Dec 6, 2018

@ameshkov

This comment has been minimized.

Copy link
Member

ameshkov commented Dec 6, 2018

There're several reasons for not moving any functionality from the main process to the content blocking extension.

  1. We are going to implement multiple content blockers in the future to overcome the Safari 50k limitation (see this comment)
  2. The main process is a single point that handles the app configuration, filters updates, and rules conversion (ag -> safari syntax), which is resource-intensive indeed.

Here is what we will do for now:

  • We will add a new checkbox to the general settings: "Show AdGuard for Safari in the menu bar"
  • If this box is unchecked, AdGuard will continue to run in the background until it's explicitly closed (via cmd+q or menu->quit)
  • "Launch AdGuard for Safari at Login" option should be duplicated to General settings as well (otherwise those who disabled menu bar icon won't be able to change it)
@Shnub

This comment has been minimized.

Copy link

Shnub commented Dec 6, 2018

Thank you, that will be helpful (even if cumbersome with the multiple extensions—but for that Apple is to be blamed).

I would propose, though, that the "Launch AdGuard for Safari at Login" option be duplicated in General Settings (and of course synchronized), rather than moved, since otherwise many users who like the menu bar icon will miss it.

@ameshkov

This comment has been minimized.

Copy link
Member

ameshkov commented Dec 6, 2018

be duplicated in General Settings (and of course synchronized), rather than moved

Yeah, you're right, thank you! I've updated the original comment

@saagarjha

This comment has been minimized.

Copy link

saagarjha commented Dec 7, 2018

So, just to be clear: if I leave that checkbox unchecked, can I use the AdGuard extension without the main app running? If so, when would I need to open the app?

@ameshkov

This comment has been minimized.

Copy link
Member

ameshkov commented Dec 7, 2018

To use AdGuard extension without the app running you should explicitly close AdGuard: menu -> close AdGuard. You'll need to run it to update filters or edit the user filter or whitelist.

@Mizzick

This comment has been minimized.

Copy link
Contributor

Mizzick commented Dec 11, 2018

Fixed

Test instruction:

  • enable/disable show adguard icon on tray in general settings.

Expected result:

  • tray icon should disappear/appear back according to current setting stay.
  • with disabled tray, the app should remain in background till it's closed explicitly. Filters update should work normally.
    After closing the app (with cmd+q or through menu) ads should be still blocked in Safari, filters update not working.
  • it's necessary to check launch at login functionality with these changes. The setting state should be equal in tray and general settings.

@zzebrum zzebrum added the [Changed] label Dec 18, 2018

timirila1 pushed a commit that referenced this issue Dec 19, 2018

timirila1 pushed a commit that referenced this issue Dec 19, 2018

Dmitry Kolyshev
Merge pull request #102 in EXTENSIONS/safari-app-extension from featu…
…re/84 to master

* commit '44c58625defff8d43aecbd5269f7fe59ee224343':
  #83 tray menu protection toggle
  #84 rework tray controller

@zzebrum zzebrum closed this Dec 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment