Open Bug 1952602 Opened 9 months ago Updated 18 hours ago

Themes do not fully work on side bar

Categories

(Firefox :: Theme, enhancement, P2)

Product:
Firefox
Component:
Theme
Version:
Firefox 136
Type:
enhancement

Tracking

()

Status:
ASSIGNED
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox-esr140 --- unaffected
firefox146 + wontfix
firefox147 + affected
firefox148 + affected

People

(Reporter: thinkseal, Assigned: emilio)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Whiteboard: [fidefe-sidebar])

Attachments

(7 files, 1 obsolete file)

what it looks like on firefox 136 stable and 138 nighlty
45.36 KB, image/png
Details
Bug 1952602 - Potential approach to improve themes with images and vertical tabs / sidebar. r=#theme!,#sidebar-reviewers!
48 bytes, text/x-phabricator-request
Details | Review
Alpenglow with vertical tabs.
158.57 KB, image/png
Details
Alpenglow with horizontal tabs.
252.21 KB, image/png
Details
chun-li-sf2-vertical-tabs-screenshot.png
1.27 MB, image/png
Details
Bug 1952602 - Implement a backwards-compatibility hack for image alignment. r=sfoster!
48 bytes, text/x-phabricator-request
Details | Review
top-100-themes.csv
11.66 KB, text/csv
Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:135.0) Gecko/20100101 Firefox/135.0

Steps to reproduce:

I installed a theme (that I had tested on Zen Browser to make sure that it had an image that would extent to the bottom of the screen if allowed) and enabled the sidebar.

Actual results:

The theme did not fully extend across the sidebar, even though it should be able to.

Expected results:

The theme image should have fully extended across the sidebar since the theme has an image that can extend that far.

The Bugbug bot thinks this bug should belong to the 'Firefox::Theme' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Theme
Flags: needinfo?(sfoster)

The sidebar does not get themed from themes. Here is an example of what it should look like with the theme I have selected.
https://github.com/Thinkseal/test/blob/main/Screenshot%202025-03-12%20at%2016.44.23.png?raw=true
And this is what it currently looks like.
https://github.com/Thinkseal/test/blob/main/Screenshot%202025-03-12%20at%2016.46.11.png?raw=true

We anticipated this issue and looked into it in bug 1893214. In summary, our documentation has long advised theme authors that they can place one or more background images behind the toolbars at the top of the browser window. We even suggest a size, typically 2000px wide and at least 200px high. The new sidebar adds a sidebar launcher - which is kind of a toolbar - below the other toolbars and on the side of the content area. Ideally we'd enable the background image to sit behind the sidebar launcher as well. But most of the existing 2000x200 size and aspect ratio theme images would not work well there. They were never designed to stretch or tile vertically. Another option would be to add the theme's background image to the sidebar so it would show up twice. That might work well for some themes and not so well for others.

The design intention was always to have a smooth seamless transition between the horizontal toolbars and the new launcher vertical toolbar. To support that I think we may have to extend the theme manifest itself and add a new property for images explicitly design to work this way.

I looked for and failed to find a bug on file for that work. Unless I just missed it, maybe this can be it?

Blocks: 1946511
Severity: -- → N/A
Status: UNCONFIRMED → NEW
Type: defect → enhancement
Ever confirmed: true
Flags: needinfo?(sfoster)
Priority: -- → P2
See Also: → 1893214
Whiteboard: [fidefe-sidebar]

The design intention was always to have a smooth seamless transition between the horizontal toolbars and the new launcher vertical toolbar. To >support that I think we may have to extend the theme manifest itself and add a new property for images explicitly design to work this way.

I looked for and failed to find a bug on file for that work. Unless I just missed it, maybe this can be it?

I would be fine with that as long as if the theme can go down all the way it can

Duplicate of this bug: 2005148

Please release a fix for 146 asap sidebar is almost impossible to read.

(In reply to Jim from comment #6)

Please release a fix for 146 asap sidebar is almost impossible to read.

Jim is referring to bug 2005148 which I think is a wontfix except for this bug. To make tracking easier, I'll close the other bug instead of marking it as a duplicate of this one.

Flags: needinfo?(ryanvm)
No longer duplicate of this bug: 2005148
Blocks: 2005148

Take it more as a feedback request as I probably need to adjust a bunch
of themes, and eventually we might want to rename some of the variables
here, but this seems to work nicely, see screenshots attached to the
bug.

Main idea is to draw the toolbox background just once, on <body>, and
then the rest falls off that.

I imagine this could regress some themes with repeating backgrounds
perhaps? But also that'd be easy to fix on the theme's end, and I
haven't found any yet.

This came to mind because the AI window wants to do something similar,
see bug 2003921. They want a radial gradient so it's harder, tho the
problem is the same.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Attached image Alpenglow with horizontal tabs. (obsolete) —

See how the image isn't chopped when it reaches the sidebar for example.

(In reply to Emilio Cobos Álvarez [:emilio] from comment #10)

Created attachment 9532515 [details]
Alpenglow with horizontal tabs.

See how the image isn't chopped when it reaches the sidebar for example.

The screenshot attached is the same as the one before :)
Can you post screenshot for the customize page?
Also how would the findbar look with this approach?

Flags: needinfo?(emilio)

d'oh

Attachment #9532515 - Attachment is obsolete: true
Flags: needinfo?(emilio)
Blocks: 2005818

This is a screenshot with the patch applied, using the chun-li-sf2-sfa-and-sf3-stages theme. Its an example of a theme that has the additional_backgrounds_tiling: [repeat] theme property. It does not go well as this image was never really meant to be tiled this way.

A few more to try:

Note that while some have background theme properties, the image seems to just be a flat color. It seems to be the rare case that the background image is actually designed to tile when the tiling/repeat property is given.

I mean, I'm sympathetic, but also, none of those themes have more than 2 users, so doesn't seem terribly concerning... I assume that wasn't an exhaustive list? can we get a list sorted by user count or something?

I'm happy to try some mitigations if we need to, but also this looks like the kind of thing that we might be able to get away with.

Flags: needinfo?(sfoster)

I still think we should only do this if absolutely necessary, but this
would restore the status quo for the themes in comment 13.

I'd note that a bunch of those themes are broken already in various ways
(invisible text in popups, invisible tabs in vertical tabs mode, ...).

Attachment #9532828 - Attachment description: Bug 1952602 - Implement a backwards-compatibility hack for image alignment / positioning. r=sfoster! → Bug 1952602 - Implement a backwards-compatibility hack for image alignment. r=sfoster!
Depends on: 2005854
Attached file top-100-themes.csv —

A report pulling a few properties out of the top 100 themes, including additional_backgrounds, additional_backgrounds_alignment, additional_backgrounds_tiling.

Flags: needinfo?(sfoster)

I went through those with vertical tabs enabled:

Improvements, even with workaround

Improvement without workaround, equal with workaround

Major regression without workaround, equal with workaround

These use centered images which without the workaround would not align to the headerbar.

Slight regression even with workaround

For these, background repeats vertically, but doesn't tile cleanly, so you can see where the repeat starts / end (but they don't look all that bad, and horizontal tabs behavior is unchanged).

Equal regardless

These have 1x1, transparent svg for some reason that does nothing?

So, assuming we land the workaround, there are:

  • No horizontal tabs regressions.
  • No major vertical tabs regressions (read: makes theme unusable). Some of the "slight regression" themes I'm a bit on the fence on whether they should be considered a regression or an improvement. But I can see the image seams so I went for the conservative option.
  • Quite a few improvements.
  • I think it generally simplifies our implementation.

So I tend to think it's worth a shot (but of course I'm biased ;))

After discussing with Sam, it might be worth reaching out to the developers of affected themes like the ones that regress in comment 17... Luca / Rob, would that be possible?

Flags: needinfo?(rob)
Flags: needinfo?(lgreco)

Thanks for looking into a potential fix Emilio.

Since the contrast problem with some themes has been an issue for a while it would be a good idea to revisit how we should handle that. We met with some folks about this earlier in the year which is documented, along with the different options in this doc. One of the options was an intervention to detect low contrast, which i don't believe has been tried yet and will likely have its own set of issues. As you've suggested, we should consider if communicating with theme authors to choose a better frame color is the best option. Adding a new manifest values is another, but then decide what the fallback should be.

We haven't really had time to revisit this due to other priorities but it might be best for the sidebar team to have a meeting with addons folks after the holidays to decide how to handle this (we can include you as well since you've helped quite a bit with this issue/sidebar styling generally).

(In reply to Emilio Cobos Álvarez [:emilio] from comment #19)

After discussing with Sam, it might be worth reaching out to the developers of affected themes like the ones that regress in comment 17... Luca / Rob, would that be possible?

Is this about reaching out to the authors of the themes from comment 17, or "all" developers?
There are over half a million static themes on AMO, but we can bring that number down considerably if we focus on themes with a X number of users.
Do we have criteria by which we can determine statically which themes need a fix?

And if we have the ability to determine statically whether a theme is "broken", can we automatically repair that theme in Firefox (or mitigate the impact), without needing theme developers to be involved (if they are still around at all!)?

Flags: needinfo?(rob)
Flags: needinfo?(lgreco)
Flags: needinfo?(emilio)

(In reply to Sarah Clements [:sclements] from comment #20)

Since the contrast problem with some themes has been an issue for a while it would be a good idea to revisit how we should handle that. We met with some folks about this earlier in the year which is documented, along with the different options in this doc. One of the options was an intervention to detect low contrast, which i don't believe has been tried yet and will likely have its own set of issues. As you've suggested, we should consider if communicating with theme authors to choose a better frame color is the best option. Adding a new manifest values is another, but then decide what the fallback should be.

But this work is tangential to that. This work mostly affects background images (and prevents them from being cropped to the headerbar). So there's a chance that for a theme with a tiled image this improves the contrast issue but shouldn't ever make it worse / different.

(In reply to Rob Wu [:robwu] from comment #21)

Do we have criteria by which we can determine statically which themes need a fix?

The idea is that with the workaround proposed, no themes need a strong fix (we're not making any theme unusable), but some themes can change rendering (sometimes for the better, sometimes slightly for the worse).

And if we have the ability to determine statically whether a theme is "broken", can we automatically repair that theme in Firefox (or mitigate the impact), without needing theme developers to be involved (if they are still around at all!)?

The workaround suggested in comment 15 fixes totally broken themes. It's hard to automatically know if the rendering change for other themes is an improvement or a regression, because it depends on the pixels of the image.

So the idea would be to ping developers or themes with theme.images.additional_backgrounds (and maybe some specific values of theme.images.additional_backgrounds_alignment only), that their theme rendering might change. Limiting it to popular themes for some definition of popular seems fine to me.

Flags: needinfo?(emilio)
Flags: needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: