[Windows 10] Addon context menu checkbox items show icons instead of checkboxes, are misaligned
Categories
(Firefox :: Menus, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox89 | --- | verified |
People
(Reporter: glandium, Assigned: zombie)
References
(Blocks 1 open bug, Regressed 1 open bug)
Details
(Whiteboard: [proton-context-menus] [priority:2b])
Attachments
(4 files)
See attached screenshot.
OS: Windows 10, arm64.
Display: 1920x1080 @ 150% scaling.
Comment 1•4 years ago
|
||
Which add-on is this (ie how can people reproduce), and what font and font size is in use here? (the browser devtools can tell you; you may want to turn on "Disable Popup Auto-Hide" in the menu there)
Edit: also what build ID of nightly is this?
Reporter | ||
Comment 2•4 years ago
|
||
about:buildconfig says
Built from https://hg.mozilla.org/mozilla-central/rev/b0d9f96ea571e00f4e0dbd78a089833a22ff2252
Addon is Rikaichamp. Installing the addon should be enough to get it in the menu. Interestingly, when the menu item is selected once to enable the feature, it shows an icon and then is aligned.
(In reply to :Gijs (he/him) from comment #1)
(the browser devtools can tell you; you may want to turn on "Disable Popup Auto-Hide" in the menu there)
I got the popup to not autohide, and got the inspector to select the item, but the font tab says there is no font used.
Comment 3•4 years ago
|
||
I'm more than happy to fix Rikaichamp if the add-on itself is to blame.
The context-menu is added here: https://github.com/birtles/rikaichamp/blob/4b6c9bec5acfdeac9dd15b6ec22b6cb8e1aca31a/src/background.ts#L416-L423
Comment 4•4 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #2)
(In reply to :Gijs (he/him) from comment #1)
(the browser devtools can tell you; you may want to turn on "Disable Popup Auto-Hide" in the menu there)
I got the popup to not autohide, and got the inspector to select the item, but the font tab says there is no font used.
Can you check the computed style view for the item? You may need to turn on browser styles with the checkbox in that view, and then use the filter box for font
.
The reason I'm asking is that there are other things that are misaligned in that menu (most obviously the icons at the top) and the menu uses em
margins/paddings in some cases, and Windows allows font and font size configuration, and some of the defaults vary across Windows locales. Because I am not seeing the other misalignment, I kind of suspect the same will be true once I test with the add-on, and there is some dependency on the system font and font-size being different for this issue to occur.
Reporter | ||
Comment 5•4 years ago
|
||
font-family: Yu Gothic UI;
font-feature-settings: normal;
font-kerning: auto;
font-language-override: normal;
font-optical-sizing: auto;
font-size: 12px;
font-size-adjust: none;
font-stretch: 100%;
font-style: normal;
font-synthesis: weight style;
font-variant: normal;
font-variant-alternates: normal;
font-variant-caps: normal;
font-variant-east-asian: normal;
font-variant-ligatures: normal;
font-variant-numeric: normal;
font-variant-position: normal;
font-variation-settings: normal;
font-weight: 400
Reporter | ||
Comment 6•4 years ago
|
||
FWIW, it still happens if I add a CSS rule via the toolbox to change the font-family to Arial on the parent menupopup.
Comment 7•4 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #6)
FWIW, it still happens if I add a CSS rule via the toolbox to change the font-family to Arial on the parent menupopup.
Thanks. Not that it matters, but the en-US default is Segoe UI
. :-)
Anyway, having tried this out, the issue is, apparently, triggered by the add-on having a menuitem of type checkbox as well as an icon, which then actually never appears. This causes a few problems:
- the icons at the top each get 36px padding which warps everything. They should be excluded from the selector at https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/toolkit/themes/windows/global/menu.css#350 . Though the menugroup should probably get that same padding, minus the padding we compute for the first item to make the icon align... Or something. It's not really clear what the best end state there would be. AFAIK it is not possible to show these navigation items together with any checkable context menu item without involving add-ons.
- the add-on item itself has no icon when not enabled. Then the icon element is sized as 0x0 but its container is still visible and still gets 12px margin, which off-sets the add-on item by those 12px compared to all the other items. From the link in comment #3 (thank you!) I don't understand off-hand why the icon is only there when enabled, or if it would be straightforward to detect this situation from within the CSS.
- I don't really understand the add-on item; it claims to be a checkbox with an icon, but AFAICT we never show a checkbox, only an icon. I don't really understand why it specifies both, or why the icon is not there when the item is "unchecked". It looks like without proton, only the checkbox shows, never the icon.
Tomislav, for (2) and (3), do you know what the expectation is for checkable add-on items?
Comment 8•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #7)
- the icons at the top each get 36px padding which warps everything. They should be excluded from the selector at https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/toolkit/themes/windows/global/menu.css#350 . Though the menugroup should probably get that same padding, minus the padding we compute for the first item to make the icon align... Or something. It's not really clear what the best end state there would be. AFAIK it is not possible to show these navigation items together with any checkable context menu item without involving add-ons.
I guess this might not be clear without more context. For checkable menuitems, you need a "gutter" in which to display the checkbox, if you want the text of the remaining items to be aligned. For stylistic reasons, with proton on Win10, we only add this spacing if there is a visible item that can be checked (for this to happen it doesn't need to be checked when the menu appears). As noted in my previous comment, the group of 4 icons at the top isn't set up to deal with this, because out of the box this situation never happens.
The constraints for the group at the top are a bit annoying to work with. We want:
- the leading edge of the first icon to be aligned with the text of the other menuitems
- the trailing space after the last icon to be identical to the leading space before the first
- the icons' visible hover boxes to be identical and centered around the icon
- no dead space between the icons
And, at least when there was no gutter, the hover box around icons should touch the edges of the menu.
With the gutter being 36px wide, that would make for very very wide hover states (36 + 16 + 36 = 88px), and thus a very wide menu. I don't think that's desirable. Amy, what would you like us to do here?
Comment 9•4 years ago
|
||
OK, per discussion with Amy, we just want to keep the navigation group looking the same, irrespective of the presence/absence of checkboxes. It'll lose horizontal alignment with the text but that's OK.
I'm still curious about the story wrt the add-on and the expectations around icon vs. checkbox, so keeping Tomislav's needinfo.
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #7)
- the add-on item itself has no icon when not enabled. Then the icon element is sized as 0x0 but its container is still visible and still gets 12px margin, which off-sets the add-on item by those 12px compared to all the other items. From the link in comment #3 (thank you!) I don't understand off-hand why the icon is only there when enabled, or if it would be straightforward to detect this situation from within the CSS.
- I don't really understand the add-on item; it claims to be a checkbox with an icon, but AFAICT we never show a checkbox, only an icon. I don't really understand why it specifies both, or why the icon is not there when the item is "unchecked". It looks like without proton, only the checkbox shows, never the icon.
Tomislav, for (2) and (3), do you know what the expectation is for checkable add-on items?
Sorry for the delay, yes I believe at least in Quantum design (ISTR it might have been different before 57), for extensions with only 1 menu item, which is a check box, we don't show the extension icon, just the check box.
Here's a screenshot of how it looks on Windows 10 checked and unchecked before Proton, and with Proton. It seems that all extension icons that were in the left gutter pre-Proton are now padded in, which is probably not what we want.
Assignee | ||
Comment 11•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #9)
OK, per discussion with Amy, we just want to keep the navigation group looking the same, irrespective of the presence/absence of checkboxes. It'll lose horizontal alignment with the text but that's OK.
Oops, I kept this open while at meetings/dinner, and didn't see this comment. I just tested without the single-checkbox-item extension Rikaichamp, and it apparently already looks like that with icons (if I'm reading the description right). I guess making the checkbox case also indent the text would be good then.
Comment 12•4 years ago
|
||
(In reply to Tomislav Jovanovic :zombie from comment #11)
Created attachment 9215563 [details]
context-menu-ext-normal.png(In reply to :Gijs (he/him) from comment #9)
OK, per discussion with Amy, we just want to keep the navigation group looking the same, irrespective of the presence/absence of checkboxes. It'll lose horizontal alignment with the text but that's OK.
Oops, I kept this open while at meetings/dinner, and didn't see this comment. I just tested without the single-checkbox-item extension Rikaichamp, and it apparently already looks like that with icons (if I'm reading the description right). I guess making the checkbox case also indent the text would be good then.
Sorry, that comment was just about the back/fwd/refresh/star icons.
I think the case without the checkbox in your screenshot does look as expected, yes. Based on your description and the pre-proton state, I guess I would make the checkbox state have a checkmark instead of the add-on icon as it does now. But if we have that gutter anyway (for checkmarks), we might as well put the icons (for non-checkmark-items) in it and align the text, it'll look less sloppy than the current state.
Assignee | ||
Comment 13•4 years ago
|
||
Gijs asked me if I can help with this, so I investigated. The main issue seems to be that pre-Proton, we used default appearance for menu checkboxes, and hid the extension icon image element. But on Windows 10 with Proton, we're crafting a custom appearance, and that seems to need the image.menu-iconic-icon
to be visible, see:
https://searchfox.org/mozilla-central/rev/759872688d/toolkit/themes/windows/global/menu.css#358,379,384
Unfortunately, the inherited image src
attribute wins over the list-style-image
css property for extension menu items, and checkbox is not rendered properly. I don't know if this can be solved through CSS somehow, but I think I could fix it in extensions framework code that generates menu items, by just not setting the image attribute in this case.
After that, this should a matter of adjusting padding and margins to properly line up everything.
Hey Molly, I think you introduced this new structure for checkboxes, does above sound reasonable, or is there a better way I'm not seeing?
Comment 14•4 years ago
|
||
Hi! Thanks for taking this on. Yes, I do think your assessment is accurate. I was not actually aware that this kind of menu item existed until I read this bug, so there's no allowance for it in the checkbox setup; it probably is easier to omit the src
as you suggest than to alter the checkbox setup to make it work.
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 15•4 years ago
|
||
-
Stop setting the image attribute for extension menu checkbox items. They
don't work with the new custom chekbox appearance CSS, and not shown anyway. -
When gutter is visible, stop padding menu items with an icon outside of it,
and use the gutter as a place for said icons. -
Adjust margin for menu items with icons to align the text with the rest of
the menu items when the gutter is visible.
Comment 16•4 years ago
|
||
Comment 17•4 years ago
|
||
bugherder |
Comment 18•4 years ago
|
||
Reproduced the initial issue using the Rikaichamp extension using old Nightly build from 2021-04-13, verified that using Firefox 89.0b1 and Rikaichamp extension the every label inside context menu is aligned correctly and a check is displayed when the extension is enabled.
Description
•