canonical / canonical.com

Repository for the new version of canonical.com
Other
33 stars 66 forks source link

bug: Replace homepage suru that was incompatible with firefox #1347

Closed petesfrench closed 1 month ago

petesfrench commented 1 month ago

Done

QA

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-9552?focusedCommentId=554655 https://github.com/canonical/canonical.com/issues/1201

webteam-app commented 1 month ago

Demo

Jenkins

demos.haus

akbarkz commented 1 month ago

@petesfrench Can you please fill in the description of this PR fully and assign someone for review?

petesfrench commented 1 month ago

@akbarkz I will when it is ready for review

akbarkz commented 1 month ago

@lyubomir-popov Can you please approve this change?

petesfrench commented 1 month ago

@nobuto-m Someone from our design team is looking further into this

nobuto-m commented 1 month ago

Hold on please. Nothing is fixed as per my comment above.

image

petesfrench commented 1 month ago

@nobuto-m Can you share the OS and browser details please?

I have tested on multiple browsers and the issue is resolved for me

nobuto-m commented 1 month ago

@nobuto-m Can you share the OS and browser details please?

I have tested on multiple browsers and the issue is resolved for me

Sure, it's in the initial description. https://github.com/canonical/canonical.com/issues/1201#issue-2179454729

The key to reproduce the issue is if the stack supports color management or not and the color space is wider than sRGB. So not every device/display can reproduce the issue.

nobuto-m commented 1 month ago

I think the solution is simple. https://github.com/canonical/canonical.com/issues/1201#issuecomment-2257884449

We can just strip the color profile from the image because the visible border happens in the context of color-management enabled JPEG vs CSS without a color management. If we use color-management stripped JPEG then we don't have to worried about the color transformation by color management at all.

lyubomir-popov commented 1 month ago

the original jpeg had no profile. but here it is again for one last attempt. I'm attaching screenshots of all relevant steps during the export, please let me know if you think other settings would work better. Caveat: it must not change the appearance on freshly downloaded and installed firefox, otherwise we'd be fixing it for one user and breaking it for most others.

colour settings: image

removed profile: image

sampling the top edge to verify the colour along the edge matches the hex value we use for the css dark background: image

export with no conversion to srgb and no profile included: image

resulting file: https://assets.ubuntu.com/v1/7e43029f-suru_main.jpg

lyubomir-popov commented 1 month ago

apologies if we've already asked, but does the problem appear with firefox updated and reset to defaults, or only after colour management settings have been altered from the defaults firefox ships with?

nobuto-m commented 1 month ago

apologies if we've already asked, but does the problem appear with firefox updated and reset to defaults, or only after colour management settings have been altered from the defaults firefox ships with?

The issue happens out of the box. By disabling Firefox's color management explicitly the issue could be masked though. https://github.com/canonical/canonical.com/issues/1201#issuecomment-2259743319

nobuto-m commented 1 month ago

resulting file: https://assets.ubuntu.com/v1/7e43029f-suru_main.jpg

This doesn't work in the same way as: https://github.com/canonical/canonical.com/issues/1201#issuecomment-2302389988 (border is not visible, which is good. But gradation is ugly)

Let me take some time to take a deeper look.

Caveat: it must not change the appearance on freshly downloaded and installed firefox, otherwise we'd be fixing it for one user and breaking it for most others.

I totally get this part. But preventing a regression is easy in this case. We can prepare a new image and deploy it to the demo site as usual. Then I can confirm the issue is fixed then somebody else like you folks can also confirm that the current site and the demo site have no difference with human eyes.

petesfrench commented 4 weeks ago

@nobuto-m Would you care to take a look at this demo @lyubomir-popov asked me to put together? https://github.com/canonical/canonical.com/pull/1360

nobuto-m commented 4 weeks ago

Sure I will come back to the topic once I sort out other stuff. I've just been hectic this week.

petesfrench commented 1 week ago

Hey @nobuto-m, were you able to have a look at the aforementioned PR?