overdodactyl / ShadowFox

A universal dark theme for Firefox
https://overdodactyl.github.io/ShadowFox/
MIT License
1.32k stars 58 forks source link

Popup menus have no border or padding, causing the first item to be clicked automatically #162

Open Mange opened 6 years ago

Mange commented 6 years ago

Firefox version: Firefox Developer Edition 61.0b10 (64-bit) Operating System: Linux Installed shadowfox: (Installed via Arch AUR)

› shadowfox-updater --version
ShadowFox updater 1.5.1
› pacman -Q shadowfox-updater 
shadowfox-updater 1.5.1-1

When I right click anywhere on the page, the first menu item is automatically chosen because it ends up right under the cursor. I can only use right click menus by holding down right click and then move the cursor to the item I want before I release.

This happens because Shadowfox removes border and padding of popup menus. I don't understand why it does that; shouldn't it only customize colors?

I fixed it by commenting out the changes to size and behavior.

diff --git a/userChrome-original.css b/userChrome-fixed.css
index 25b6901..77451b9 100644
--- a/userChrome-original.css
+++ b/userChrome-fixed.css
@@ -133,10 +133,10 @@ menupopup > menu > menupopup,
 menupopup scrollbox,
 popup,
 popup > menu > menupopup {
-  -moz-appearance: none!important;
+  /* -moz-appearance: none!important; */
   background: var(--in-content-box-background)!important;
-  border: none!important;
-  padding: 0!important
+  /* border: none!important; */
+  /* padding: 0!important */
 }
 menu.subviewbutton > .menu-right {
   fill: #000!important
@@ -154,16 +154,16 @@ menu.subviewbutton > .menu-right {
 }
 menuitem,
 menupopup menu {
-  -moz-appearance: none!important;
+  /* -moz-appearance: none!important; */
   color: var(--in-content-selected-text)!important;
   background: var(--in-content-box-background)!important
 }
 menupopup menuseparator {
-  -moz-appearance: none!important;
-  padding: 1px!important;
-  margin: 5px 0!important;
+  /* -moz-appearance: none!important; */
+  /* padding: 1px!important; */
+  /* margin: 5px 0!important; */
   background: var(--in-content-table-border-dark-color)!important;
-  border-top: none!important
+  /* border-top: none!important */
 }
 #context-navigation menuitem[disabled=true],
 menu[disabled=true],

Screenshot before

before

Mouse cursor is on the origin pixel of the first item (0×0) so it is selected on button release.

Screenshot after

after

Mouse cursor ends up on the border so nothing is selected.

Mange commented 6 years ago

EDIT: Last screenshot added and version of Firefox specified in the OP.

Let me know if you need to know anything else about my setup. :-)


My personal (humble) opinion on that border: The light border around the popup is necessary in case you right click in a dark area, or else you cannot tell where the menu is. I don't think it is wise to remove it. Changing the color using border-color, I would understand, but removing it completely seems like a mistake to me.

CaptaPraelium commented 6 years ago

I can't reproduce this problem here. When I open the context menu, the first item is not selected. Latest nightly.

I don't have any issue identifying the content of the menu from the page, and prefer the flat borderless design.

Mange commented 6 years ago

I can't reproduce this problem here. When I open the context menu, the first item is not selected. Latest nightly.

Maybe the GTK theme has an effect on this. :thinking:

I think I'm on the Adwaita theme, but I'm not sure how to tell since I'm not running GNOME.

I don't have any issue identifying the content of the menu from the page, and prefer the flat borderless design.

Don't you think it looks weird with no spaces around the icons in the menu? (Like for Bitwarden and Take a Screenshot) Anyway, I get that this is a matter of taste and that everyone's different.

CaptaPraelium commented 6 years ago

@I'm on windoze right now and I think that might have something to do with it. We don't have the border but do have the borders around icons and spacing between menus.... A screenshot (this is your screenshot above, my menu is bottom-right)

image

Mange commented 6 years ago

Aha, yes that makes sense. There are Windows-specific media queries that add padding again after removing them for the generic case.

overdodactyl commented 6 years ago

Hi @Mange, thanks for reporting this issue, and my apologies for any troubles this has caused you!

I appreciate the included fix as well, that will help me figure out what the best solution here is.

Context menus have given me more trouble than any other aspect of this project. There seems to be a lot of variation from OS to OS and I'm currently limited in what I can test on.

I'll take this and all the feedback in #107 into account and try and figure out a better approach. Probably will just have to deal with more media queries.

I definitely agree though, the screenshots and problem you described are far from ideal

gombosg commented 6 years ago

I'm experiencing the same - no padding on Fedora 28, FF61 after installing ShadowFox. The fix recommended by @Mange did work. (Won't attach screenshot, it's the same...)

arguablykomodo commented 6 years ago

I think that #107 should be closed in favour of this issue

Also here are some more comparison screenshots:

Before Shadowfox: before_shadowfox

After Shadowfox: after_shadowfox

I am running Firefox 62 on Ubuntu 18.04 (using the arc-dark theme, hence the blue-ish background on the before picture)

overdodactyl commented 6 years ago

Sorry I've let this issue become stale.

I'll close #107 and try and install some VMs tonight and try and sort out a solution that works across all platforms.

overdodactyl commented 6 years ago

Alright, so it looks like Firefox has changed a good amount of the code relating to context menus, so I was able to remove a large chunk of the css I was using (including the padding and height definitions I think were causing the problems across different OS's).

I've only tested it on macOS, so if Windows and Linux user's could let me know if this fixes things (and hopefully doesn't cause new ones), that would be great!

DemonicSavage commented 6 years ago

I've tested this commit on Linux. Didn't work, I'm still having the problem.

overdodactyl commented 6 years ago

Thanks for testing @ghuwe. I'm a little stumped on what to do here - it seems like to fix the problem on linux the -moz-appearance: none!important; lines need to be removed.

Unfortunately, these seem to be needed to adjust the background colors on macOS and I'm not aware of any OS specific media queries other than windows ones.

I'll keep digging..

SW1FT commented 6 years ago

Well, this is how it looks now on Windows 10. context

overdodactyl commented 6 years ago

I reverted back to the original styling until a better fix is found