Fix 1822: Sync container colors with Firefox container settings#2841
Fix 1822: Sync container colors with Firefox container settings#2841nekno wants to merge 2 commits intomozilla:mainfrom
Conversation
… the current text color in both light and dark modes, as it does in the Firefox container settings.
| --inactive-opacity: 0.3; | ||
| --overflow-size: 1px; | ||
| --icon-fit: 8; | ||
| --icon-fit: 9; |
There was a problem hiding this comment.
This seems to just be hardcoded to match the number of colors. Each color swatch is 32 x 32px, and the panel is 320px wide, so you can fit 10 colors in a row without compromising on minimum spacing/padding on anything.
This changes it from 8 to 9 so all colors fit on the same row, and the minimum spacing is still met.
| --identity-tab-color: currentColor; | ||
| --identity-icon-color: currentColor; | ||
| } | ||
|
|
There was a problem hiding this comment.
This matches the Firefox stylesheet for the ContextualIdentity component.
| <label for="edit-container-panel-choose-color-${containerColor}" class="usercontext-icon choose-color-icon" data-identity-icon="circle" data-identity-color="${containerColor}">`; | ||
| }; | ||
| const colors = ["blue", "turquoise", "green", "yellow", "orange", "red", "pink", "purple"]; | ||
| const colors = ["blue", "turquoise", "green", "yellow", "orange", "red", "pink", "purple", "toolbar"]; |
There was a problem hiding this comment.
This matches the toolbar color named in the Firefox container settings dialog.
|
@nekno As I could understand from you commit you just added support of But maybe it is not a bug but it is the special intention of developers to give no chance to mislead users by providing possibility to select Be sure as anybody will select
I'm really not sure. Maybe we need ask somebody like @dannycolin. |
|
@achernyakevich-sc — I don't know the history why the color is named The color named I added screenshots to the description to show it rendered in context. |
|
Then it looks reasonable. Thank you for your efforts. |
|
Sorry for the late response. There was some issues/limitations with the toolbar color that prevented it to be added in the list. See a previous PR: #2192 |
|
Thanks, @dannycolin, I didn't find that previous PR in my searching. I wish I would have! It's funny that we came up with almost identical PRs independently, though I used In testing, I see your point still stands, that None of the colors in MAC are expressly themable, so despite the While I would love to follow your suggestion in the other PR and fix this upstream, the effort and knowledge required to add a custom color picker is much greater than what's required to fix this issue. If you have any references on how to pull in an existing color picker and incorporate it into the experience, I'm willing to try, but I'm likely not the best person. I will be learning most of it from scratch and the juice is not likely worth the squeeze. I ask honestly — must we suffer this issue another 5 years until someone with the requisite skills and knowledge finds the time? |
This isn't necessarily true because we can't control third-party themes.
This would be the best longterm solution but there are a lot of moving pieces. The mechanism needs to be implemented in Firefox itself. Plus, this change would have an impact on the webextension API so we could potentially break other people addons if their code expect hardcoded color values. I'm not a big fan of adding more hardcoded colors but I can still ask folks at Mozilla if we couldn't find a short term solution. For example, we could align our color values with the new Tab Group feature which offers 9 predefined colors. It always offers variant of the color palette depending if the theme is set as a dark or light theme. In other words, this solution would be similar to your fix for the end-user but we'd be in control of the visual rendering which would avoid the pitfalls of using the toolbar color. I'll keep you posted. |



npm testand all tests passed.Description
This PR fixes #1822, which captures the issue well.
Firefox allows 9 container colors, including a
toolbarcolor that matches the current text color in light and dark modes.The extension currently only supports 8 of those colors, omitting the
toolbarcolor. That allows users to set a color in the Firefox container settings that doesn't appear or allow editing properly in the extension container settings.This PR adds support for the
toolbarcolor, to sync properly with the color names in the Firefox container settings.I assume this issue has remained unresolved for ~6 years and marked
Needs: Mozilla Centralbecause, in lieu of hardcoding two identical collections across codebases, it would be best, architecturally, to expose the array of supported colors and icons onContextualIdentityServicein Firefox as the source of truth, and then consume those in the extension, so they would never be out-of-sync. There are also the separate requests to support more icons and any arbitrary RGB color.Both of those changes would require a lot more support, so I figured it would be better to get this fixed first using the current approach, and those alternatives would be better taken on by a member of the project(s) rather than a random outside contributor.
If those alternatives are something you're looking for someone to take on, let me know.
Type of change
Select all that apply.
Tag issues related to this pull request: