Reader View sidebar (and perhaps the settings panel?) is too "bright" in dark mode

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
normal
RESOLVED FIXED
4 years ago
a month ago

People

(Reporter: supaiku, Assigned: akshithashetty84)

Tracking

42 Branch
mozilla68
Points:
---

Firefox Tracking Flags

(firefox68 fixed)

Details

(Whiteboard: [about-reader-ui])

Attachments

(2 attachments)

Reporter

Description

4 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150725030209

Steps to reproduce:

1. Enter reader view
2. Set background to dark




Actual results:

The panel on the right stays white. See attachment.


Expected results:

The panel should adopt the color of the background, because the way it's now, it's too distracting when reading on a dark background color.

Updated

4 years ago
Component: Untriaged → Reader Mode
Product: Firefox → Toolkit

Updated

3 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Summary: Reader View panel is too "bright" in dark mode → Reader View sidebar (and perhaps the settings panel?) is too "bright" in dark mode

Updated

3 years ago
Whiteboard: [about-reader-ui]

Updated

a year ago
Duplicate of this bug: 1350528

Updated

a year ago
Duplicate of this bug: 1444568

Comment 3

4 months ago

Is there a chance that I can be assigned to this Issue please?

Comment 4

4 months ago

(In reply to Victor from comment #3)

Is there a chance that I can be assigned to this Issue please?

This isn't a mentored bug... are you working on a patch for this bug?

Flags: needinfo?(vivee18)

Comment 5

3 months ago

yes I am working on a patch sir. But I hope I can continue?

Flags: needinfo?(vivee18)

Comment 6

3 months ago

(In reply to Victor from comment #5)

yes I am working on a patch sir. But I hope I can continue?

I'm always happy with patches. But I'd like to understand how you intend to fix this. Can you outline what your plan is?

I'll note that there's a plan to do a Google Summer of Code project around this code area, see https://wiki.mozilla.org/Community:SummerOfCode19 . Based on those UX mockups ( https://mozilla.invisionapp.com/share/87N9YQYCTJZ#/screens/315073983_Reader_View_Redesign ) the code might change substantially, so I'm not sure if now is the best time to work on this issue; the code might have to be rewritten soon...

Flags: needinfo?(vivee18)
Comment hidden (off-topic)

Updated

3 months ago
Duplicate of this bug: 1532137
Comment hidden (off-topic)
Comment hidden (off-topic)
Comment hidden (off-topic)

Comment 12

3 months ago

(In reply to :Gijs (he/him) from comment #10)

(In reply to zaraahmad2121 from comment #9)

Hi, I am an applicant for GSoC. I am currently in my senior year of BS-CS programme. I had like to work on this project "Firefox Reader Redesign". Could you please guide me and help me get started?

Victor has already indicated he wants to work on this bug. Maybe have a look at bug 1151735 instead.

(In reply to Berkay Barlas from comment #7)

Given this bug already had interest, perhaps look at bug 1151732.

Thanks for considering granting me the privilege, I also am a gsoc applicant but i was with thought that there was a person already working on the issue, I have tried checking @Mozilla-central repo and found this component (Reader.js) https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/Reader.js

However I am queit new to the large codebase and will need guidance to where i can look to be sure i am doing the right thing and to see if another was working already on it for gsoc, Thanks for reading too. I'm quiet ready for your reply to get started.

Flags: needinfo?(vivee18)

Comment 13

3 months ago

(In reply to Victor from comment #12)

Thanks for considering granting me the privilege, I also am a gsoc applicant but i was with thought that there was a person already working on the issue, I have tried checking @Mozilla-central repo and found this component (Reader.js) https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/Reader.js

However I am queit new to the large codebase and will need guidance to where i can look to be sure i am doing the right thing and to see if another was working already on it for gsoc, Thanks for reading too. I'm quiet ready for your reply to get started.

The CSS is at https://searchfox.org/mozilla-central/source/toolkit/themes/shared/aboutReader.css . If we wanted to add styling such that the sidebar also goes dark when the background goes dark, that's where it'd need to happen. We might need to also update the icons referenced there so they use bright foreground colours instead of dark ones when the background is dark.

Do you have a copy of Firefox built from source already? If not, that'd be a good place to start. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds and https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build can help.

Comment 14

3 months ago

Hi Victor, I found some helpful things to simplify the large codebase for you. Might be of help :)

This is how I would have gone about it:

  1. To update the color of the toolbar: Try to figure out how the background change is triggered on the click of sepia/dark/light button. I found these separate classes body.sepia and body.dark which change the background colour in https://searchfox.org/mozilla-central/source/toolkit/themes/shared/aboutReader.css.
    I believe some new code has to be added under (Line 126):
    /* Override some controls and content styles based on colour scheme */
    :Gijs, Let me know if I am going wrong here.
  2. Change the icons/ add icons of a lighter color for dark mode, you can use Inkscape for that. They can be found using inspect element.
  3. Update the dropdown pop-up to match the color of the theme.
  4. The dropdown arrow has a white triangle, which looks weird when the dropdown popup has a different color. So you should make that triangle transparent.

Comment 15

3 months ago

:Gijs, In case Victor is busy or gives up working on the issue, then I would like to work on it. I am a GSoC applicant. Thanks! :)

Comment 16

2 months ago

Victor, any progress here? Are you stuck on anything I can help with?

Flags: needinfo?(vivee18)

Comment 17

2 months ago

(In reply to :Gijs (he/him) from comment #13)

The CSS is at https://searchfox.org/mozilla-central/source/toolkit/themes/shared/aboutReader.css . If we wanted to add styling such that the sidebar also goes dark when the background goes dark, that's where it'd need to happen. We might need to also update the icons referenced there so they use bright foreground colours instead of dark ones when the background is dark.

Do you have a copy of Firefox built from source already? If not, that'd be a good place to start. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds and https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build can help.

:Gijs (he/him), I assume one has to build entire Firefox source code to make changes(to .js files) in order to test it. Is this right or is there a faster way to go about it?

I have been tring to build Firefox for a few days now,

  1. I started off with trying to build it on my MacBook Air. Some errors are coming related to inttype.h not found. It seemed to be an error due to Mojave SDK, but even installing older SDK did not work. I did not have much luck in resolving the issue. I even tried asking for help on #build in Mozilla irc.
  2. Further, I tried it on a few other college systems with no luck.
  3. I moved on to trying it on GCP (The Windows VM always fail during hg clone, and with a Linux VM I fixed a few library issues and currently trying to build)
  4. I also tried Janitor
    I have not been able to do much development as of now. A lot of times the hg clone aborts in the middle. I was able to compile Firefox a couple of times on Janitor but now I ran into some Janitor specific bug.

I am confused on what would be the best way to go about this, could you please help?

Comment 18

2 months ago

(In reply to sonali18317 from comment #17)

:Gijs (he/him), I assume one has to build entire Firefox source code to make changes(to .js files) in order to test it. Is this right or is there a faster way to go about it?

You can use artifact builds, to download the binary (C++, Rust, some other) bits instead of compiling them. See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds . Basically, you want to edit your .mozconfig file (create one if you haven't yet) to specify the flags mentioned there, and then ./mach clobber and ./mach build, and that should be a lot easier, especially on your macbook air (you still need mozillabuild bits on windows).

For this bug though, I'd like to give Victor a chance to respond for a bit longer.

Comment 19

2 months ago

:Gijs, Thanks that helped a lot.
No problem, I am working on Bug 1151735

Assignee

Comment 20

2 months ago

Hi,
Is this bug available to work on ? I would love to propose some suggestions for this in case no one has set their hands on it yet.
Thanks.

Flags: needinfo?(gijskruitbosch+bugs)

Comment 21

2 months ago

(In reply to akshitha shetty from comment #20)

Hi,
Is this bug available to work on ? I would love to propose some suggestions for this in case no one has set their hands on it yet.
Thanks.

Sure. We probably want to use the same background for the sidebar as for the main page, and make sure the icons and text in the sidebar use the same foreground colour as the rest of the page. See also comment #6.

Flags: needinfo?(vivee18)
Flags: needinfo?(gijskruitbosch+bugs)
Assignee

Comment 22

2 months ago

Thanks a lot for the direction.
So considering the mockups, the plan is to follow similar colors as proposed in the latest mockups but to to keep the icon design same as it is right now with only a change in the icon colors. What this will do is that it will keep it consistent with the existing toolbar in the light and sepia mode so that we get it working now as well as further improvise on it in the future months.
Let me know if this is this is right approach. Thank you.

Comment 23

2 months ago

(In reply to akshitha shetty from comment #22)

Thanks a lot for the direction.
So considering the mockups, the plan is to follow similar colors as proposed in the latest mockups but to to keep the icon design same as it is right now with only a change in the icon colors. What this will do is that it will keep it consistent with the existing toolbar in the light and sepia mode so that we get it working now as well as further improvise on it in the future months.
Let me know if this is this is right approach. Thank you.

Yep, this seems OK.

Assignee

Comment 24

2 months ago

Hi, so I have worked around the css involved in solving the issue and am attaching a video of the output. (Note - The red hover on the close button is exactly the same as the present red and seems darker due to screen capturing). I have ensured that the styling is applied only for the dark mode.

Link to Video -
http://dw.convertfiles.com/files/0583129001554243057/simplescreenrecorder-2019-04-03_03.17.36.m4v

The next step would we to change the icons. I have noticed that the icon are in svg format. Could you please guide me as to how I should go about replacing these. I can create the icons myself in illustrator or any preferred software in needed. Would just need a little guidance here before I get started if any guidelines have to be maintained for the same.
Thanks for your effort on this.

Attachment #9056160 - Attachment description: Toolbar too bright in Reader dark mode → Bug 1187696 - Toolbar too bright in Reader dark mode

Comment 26

a month ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8ab68eea15b8
Toolbar too bright in Reader dark mode r=Gijs

Comment 27

a month ago
bugherder
Status: NEW → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → akshithashetty84
You need to log in before you can comment on or make changes to this bug.