Closed Bug 1703154 Opened 3 years ago Closed 3 years ago

about:devtools - white dots on RDM icon when on Dark mode

Categories

(DevTools :: General, defect, P5)

defect

Tracking

(firefox-esr78 wontfix, firefox87 wontfix, firefox88 wontfix, firefox89 verified, firefox90 verified)

VERIFIED FIXED
90 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox87 --- wontfix
firefox88 --- wontfix
firefox89 --- verified
firefox90 --- verified

People

(Reporter: cfogel, Assigned: b19208)

References

Details

(Keywords: good-first-bug)

Attachments

(3 files, 1 obsolete file)

Attached image RDM.png

Affected versions

  • 78.9.0esr, 87.0, 88.0b7, 89.0a1(2021-04-03)

Affected platforms

  • Windows 10, Ubuntu 20;

Steps to reproduce

  1. OS and Firefox set on dark theme;
  2. Access about:devtools
  3. Scroll down and observe the icons;

Expected result

  • icons are properly displayed;

Actual result

  • white dots at the bottom of the Responsive Design mode;

Regression range

  • not a regression;

Additional notes

  • attached screenshot with the issue;
  • remains to confirm on macOS.
Has STR: --- → yes
See Also: → 1703165

The Bugbug bot thinks this bug should belong to the 'DevTools::Responsive Design Mode' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: General → Responsive Design Mode
Component: Responsive Design Mode → General

Cristian, how do you set OS dark theme on Win 10?

Flags: needinfo?(cristian.fogel)
  • Right Click on Desktop;
  • Personalize;
  • In the Settings window choose Colors;
  • In the Choose your color dropdown pick Dark.
Flags: needinfo?(cristian.fogel)

Thank you!

I can reproduce this issue on my machine.

This is because the background color of the mobile-device-image (in the SVG) is set to white to cover the window-image behind it.
It looks like the window-image should not render the part that's hidden behind the mobile-device-image and the mobile-device-image should be transparent just like the window-image.

The image is located here:
https://searchfox.org/mozilla-central/source/devtools/startup/aboutdevtools/images/feature-responsive.svg

And rendered in this file:
https://searchfox.org/mozilla-central/rev/21110f35dbb95d3c41c8c5bd363ec689900af30f/devtools/startup/aboutdevtools/aboutdevtools.js#102

I am marking this as first-good-bug for someone who's good with editing SVG images.

We could also use CSS styles within SVG reflecting the current theme (OS, Firefox) , see some comments in bug 1703451

Honza

Keywords: good-first-bug
Priority: -- → P3

Hello. I am an outreachy applicant. Can I take up this bug? Would like to contribute with some guidance from your side.

Thank you for helping with this, assigned to you.

You might want to read the docs first
https://firefox-source-docs.mozilla.org/devtools/

Honza

Assignee: nobody → b19208
Status: NEW → ASSIGNED

Hello Honza!
I am new to svg and actually got to know about it due to this bug. So, I couldn't figure out how we can not render the window part behind the mobile-device-image and make the white background transparent, as you said. However, from the fill color, I could find out the correct path tag. I noticed that the end part of that path forms that rectangular background and I just reduced its height.
So, the background appearing like dots has vanished. Have also attached the screenshot. So, it's serving the purpose. Does this work?

Thanks

Thanks Vaidehi, this sounds like it could be good enough solution.

Can you please post a patch using Phabricator?
I can do review/testing.

You might read about Phabricator here
https://firefox-source-docs.mozilla.org/mobile/android/geckoview/contributor/geckoview-quick-start.html

Thank you.

Flags: needinfo?(b19208)

Comment on attachment 9218063 [details]
White dots vanished due to reduced height of the background

Please don't use this flag, review should be done using Phabricator.

Attachment #9218063 - Flags: review+

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #9)

Thanks Vaidehi, this sounds like it could be good enough solution.

Can you please post a patch using Phabricator?
I can do review/testing.

You might read about Phabricator here
https://firefox-source-docs.mozilla.org/mobile/android/geckoview/contributor/geckoview-quick-start.html

Thank you.

Sure. I will post a patch.

Flags: needinfo?(b19208)

(In reply to Vaidehi from comment #12)

Created attachment 9218418 [details]
Bug 1703154 - [devtools] Reduced the rectangular white backgroud of mobile-device-image. r=Honza

Depends on D113386

Please ignore this patch.

(In reply to Vaidehi from comment #14)

Depends on D113386

Please ignore this patch.

Done (I marked it as obsolete)

When making changes to the patch use: hg commit --amend to work on existing one instead of introducing a new one.

Just commented in Phab.

Honza

Flags: needinfo?(b19208)
Attachment #9218496 - Attachment description: Bug 1703154 - Reduced the height of rectangular white background of mobile-device-image. r=Honza → Bug 1703154 - Reduce the height of rectangular white background of mobile-device-image. r=Honza
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/56ba7d91ec33
Reduce the height of rectangular white background of mobile-device-image. r=Honza
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

Okay.
So, I think, I don't need to push the changes as it has been done automatically.
Thank you Honza for assisting me in my first ever contribution!

Flags: needinfo?(b19208)

The patch landed in nightly and beta is affected.
:b19208, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(b19208)

Comment on attachment 9218496 [details]
Bug 1703154 - Reduce the height of rectangular white background of mobile-device-image. r=Honza

Beta/Release Uplift Approval Request

  • User impact if declined: In dark mode, white dots would appear below the "Responsive design mode" svg image
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small svg fix to just reduce height of white rectangular background
  • String changes made/needed: no
Attachment #9218496 - Flags: approval-mozilla-beta?

Tested in Nightly and works for me.

@Vaidehi, thank you for working on this.

Honza

Comment on attachment 9218496 [details]
Bug 1703154 - Reduce the height of rectangular white background of mobile-device-image. r=Honza

Polish, micropatch to an svg image, we are still in early betas, approved for 89 beta 10, thanks.

Attachment #9218496 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

The issue is verified fixed using Fx89.0b10 and the latest Fx90.0a1 on Windows 10 and Ubuntu 18.04.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: