Issue #1596, #1720 - Make status bar menu consistent with plugin abilities

src/plugin/PluginClass.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 1110 matching lines @@
 
   MENUITEMINFOW miiSep = {};
   miiSep.cbSize = sizeof(miiSep);
   miiSep.fMask = MIIM_TYPE | MIIM_FTYPE;
   miiSep.fType = MFT_SEPARATOR;
 
   CPluginClient* client = CPluginClient::GetInstance();
   CPluginSettings* settings = CPluginSettings::GetInstance();
   {
     ctext = dictionary->Lookup("menu", "menu-disable-on-site");
-    // Is domain in white list?
     ReplaceString(ctext, L"?1?", client->GetHostFromUrl(url));
-    if (!GetTab()->CanDisableOnSite())
-    {
+    /*
+     * The display state of the "disable on this site" menu item depends upon tab content
+     */
+    if (!GetTab()->IsPossibleToDisableOnSite())
+    {
+      // Since we can't disable the present content,
+      // it makes no sense to offer the user an option to block it.
       fmii.fState = MFS_UNCHECKED | MFS_DISABLED;

[Oleksandr, 2015/12/17 12:59:38]

If it is not possible to disable on site, it also means that we are already
disabled there (we don't intercept non-http(s) links). So wouldn't it make more
sense to make this: fmii.fState = MFS_CHECKED | MFS_DISABLED;?

[Eric, 2015/12/17 13:40:39]

The meaning of the check mark to me is "there is a custom override rule to
disable on this site". Since we're disabling these URL's by category, there's no
user override at play.

Also, a check mark generally indicates something you can tick on and tick off.
Putting up a check mark that the user can't uncheck breaks an ordinary
expectation of what those mean.

[sergei, 2015/12/21 10:26:23]

I agree, it should be rather checked.
Why do you think so? The meaning of this check mark is that ABP is disabled on
this particular site and there is no any connection between the way or reason
ABP is disabled, whether it's a custom override rule or a rule from some
subscription or it's hard-coded in the plugin. Currently the user can interpret
it as ABP does work on such pages and cannot be disabled.
So, not putting up a check mark that user cannot check also breaks the
expectation, doesn't it? Actually, it does not break the expectation because
it's well known fact that control can have a disabled state which means the user
is not allowed to change it.

[Eric, 2015/12/21 13:11:41]

Please feel free to bring up these points as comments to #3428, where the UI
discussion now belongs.

     }
     else if (client->IsWhitelistedUrl(GetTab()->GetDocumentUrl()))
     {
+      // Domain is in white list, indicated by a check mark
       fmii.fState = MFS_CHECKED | MFS_ENABLED;
     }
     else
     {
       fmii.fState = MFS_UNCHECKED | MFS_ENABLED;
     }
     fmii.fMask = MIIM_STRING | MIIM_STATE;
     fmii.dwTypeData = const_cast<LPWSTR>(ctext.c_str());
     fmii.cch = static_cast<UINT>(ctext.size());
 
@@ skipping 488 matching lines @@
   s_criticalSectionLocal.Unlock();
 
   return icon;
 }
 
 ATOM CPluginClass::GetAtomPaneClass()
 {
   return s_atomPaneClass;
 }