ShadowFox icon indicating copy to clipboard operation
ShadowFox copied to clipboard

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

Open Mange opened this issue 6 years ago • 14 comments

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 avatar Jun 06 '18 19:06 Mange

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.

Mange avatar Jun 06 '18 19:06 Mange

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.

CaptaPraelium avatar Jun 06 '18 20:06 CaptaPraelium

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.

Mange avatar Jun 06 '18 20:06 Mange

@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

CaptaPraelium avatar Jun 07 '18 06:06 CaptaPraelium

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

Mange avatar Jun 07 '18 14:06 Mange

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

overdodactyl avatar Jun 13 '18 20:06 overdodactyl

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...)

gombosg avatar Jul 03 '18 09:07 gombosg

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)

arguablykomodo avatar Sep 26 '18 21:09 arguablykomodo

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 avatar Sep 26 '18 23:09 overdodactyl

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!

overdodactyl avatar Sep 27 '18 02:09 overdodactyl

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

DemonicSavage avatar Sep 27 '18 04:09 DemonicSavage

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..

overdodactyl avatar Sep 27 '18 05:09 overdodactyl

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

SW1FT avatar Sep 27 '18 17:09 SW1FT

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

overdodactyl avatar Sep 27 '18 18:09 overdodactyl