Closed Bug 1443396 Opened 6 years ago Closed 6 years ago

about:memory has poor colours on some desktops

Categories

(Toolkit :: about:memory, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: pbone, Assigned: pbone)

References

Details

(Keywords: access)

Attachments

(3 files, 2 obsolete files)

Attached image about-memory.png
The about:memory page seems to specify a foreground text colour but no background colour, when the user's desktop is using a high contrast inverted theme the text is illegable.

I'm using Linux Mint with one of the dark desktop themes.

I didn't notice this problem until I tried a new profile without changing the settings I typically override.
Paul, do you have a recommendation for what we should set this to? Or do you have recommendations on changes we could make to make the page work better for high contrast users in general?
Component: Memory Allocator → about:memory
Flags: needinfo?(pbone)
Product: Core → Toolkit
The browser has some preferences (in about:preferences) for default colours.  It should be using these, I suspect it is deciding on foreground and background colour separately.

I talk about these preferences in my blog post: http://paul.bone.id.au/2017/11/26/how-i-see-the-web/, however they were set to the defaults for this bug/screenshot as it was a new profile.

Where is this part of the code, I can take a look myself if you like.
Flags: needinfo?(pbone)
(In reply to Paul Bone [:pbone] from comment #2)
> The browser has some preferences (in about:preferences) for default colours.
> It should be using these, I suspect it is deciding on foreground and
> background colour separately.
> 
> I talk about these preferences in my blog post:
> http://paul.bone.id.au/2017/11/26/how-i-see-the-web/, however they were set
> to the defaults for this bug/screenshot as it was a new profile.
> 
> Where is this part of the code, I can take a look myself if you like.

https://searchfox.org/mozilla-central/source/toolkit/components/aboutmemory/content

I'm assuming just the css file needs to change.
So I noticed that about:performance doesn't have the same problem.  Without custom settings it is black-text-on-white-background.  While this doesn't work for me and the plugin I've started using can't change the styles of about: pages, it is better than the grey-on-grey in the screenshot.  Is that what we should be going for?

At this stage, if I (with my eyesight) want to read any of the about: pages, I need to change my colour settings, which now that I've started using a plugin to adjust colours of most web pages is inconvenient.  Should this be a new bug?  (I guess there's a good reason why plugins can't change the style of the about: pages), and I'm not sure what else to suggest.
Assignee: nobody → pbone
Status: NEW → ASSIGNED
Keywords: access
Attachment #8956712 - Flags: review?(n.nethercote)
Attachment #8956713 - Flags: review?(n.nethercote)
I'm really no expert about CSS and colours in Firefox UI. The hardcoding of colours seems dodgy, though.

dolske, is this something you know about? Or can you recommend someone? Thanks.
Flags: needinfo?(dolske)
Yeah, I'd agree, I'd prefer them to be set in some kind of themes.

It seems that some other pages are set this way, like about:performance which uses a common stylesheet which looks like it gets customised depending on what platform you're on.

I might have made my patch use this stylesheet also, however since there were already hard coded colours in the file for various types of markup I didn't unless I could confidently map each of those onto something from the common stylesheet.

Cheers.
Flags: needinfo?(dolske) → needinfo?(jaws)
Comment on attachment 8956712 [details] [diff] [review]
Bug 1443396 (Part 1) - Use consistent colours on about:memory

Review of attachment 8956712 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for catching this! The notes below should help you fix up the patch.

::: mobile/android/themes/core/aboutMemory.css
@@ +9,5 @@
>   */
>  
>  html {
> +  background: #fff;
> +  color: #000;

This should continue to use `background: -moz-Dialog;` for the background-color. In addition, we can set `color: -moz-DialogText;`.

You can read more about the system color extensions at https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Mozilla_System_Color_Extensions

@@ +31,4 @@
>    margin: 1em 0em;
>    border: 1px solid ThreeDShadow;
>    border-radius: 10px;
> +  background: #ebebeb;

background: -moz-Field;
color: -moz-FieldText;

@@ +40,4 @@
>    margin-top: 0.5em;
>    border: 1px solid ThreeDShadow;
>    border-radius: 10px;
> +  background: #ebebeb;

background: -moz-Field;
color: -moz-FieldText;

::: toolkit/components/aboutmemory/content/aboutMemory.css
@@ +9,5 @@
>   */
>  
>  html {
> +  background: #fff;
> +  color: #000;

Same remarks for this file as the /mobile file.
Attachment #8956712 - Flags: review?(n.nethercote) → review-
Comment on attachment 8956713 [details] [diff] [review]
Bug 1443396 (Part 2) - Set the favicon for the about:memory page

Review of attachment 8956713 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/aboutmemory/content/aboutMemory.xhtml
@@ +7,5 @@
>  <html xmlns="http://www.w3.org/1999/xhtml">
>    <head>
>      <meta name="viewport" content="width=device-width"/>
> +    <link rel="icon" type="image/png" id="favicon"
> +        href="chrome://branding/content/icon32.png"/>

For these types of "internal" pages, see also about:config, we have stayed away from setting the favicon because many developers prefer to see more of the tab title in the tabbar. Lots of "firefox" icons can make it hard to find which tab the developer is looking for.
Attachment #8956713 - Flags: review?(n.nethercote) → review-
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> Comment on attachment 8956712 [details] [diff] [review]
> Bug 1443396 (Part 1) - Use consistent colours on about:memory
> 
> ::: mobile/android/themes/core/aboutMemory.css
> @@ +9,5 @@
> >   */
> >  
> >  html {
> > +  background: #fff;
> > +  color: #000;
> 
> This should continue to use `background: -moz-Dialog;` for the
> background-color. In addition, we can set `color: -moz-DialogText;`.

I'd normally agree, however elsewhere in this file text colours such as #400 are used and meant to contrast with a light background.  When -moz-Dialog is dark, as it sometimes is, these colours will not contrast and it will look like the attached screen shot (but in fewer places, since at least the default text colour could be improved).

We either change those other constants to mozilla system colours like in your link, which I didn't do because I don't want to remove any potential meaning that the original author had with specific colours?  Nicholas, do the colours (on most people's desktops) have any significance?

Or replace all the special values with constants as this patch has done.

I'm not sure which option is best, I don't trust my judgement when it comes to UIs generally.

Thanks.
Flags: needinfo?(n.nethercote)
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #10)
> Comment on attachment 8956713 [details] [diff] [review]
> Bug 1443396 (Part 2) - Set the favicon for the about:memory page
> 
> Review of attachment 8956713 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/aboutmemory/content/aboutMemory.xhtml
> @@ +7,5 @@
> >  <html xmlns="http://www.w3.org/1999/xhtml">
> >    <head>
> >      <meta name="viewport" content="width=device-width"/>
> > +    <link rel="icon" type="image/png" id="favicon"
> > +        href="chrome://branding/content/icon32.png"/>
> 
> For these types of "internal" pages, see also about:config, we have stayed
> away from setting the favicon because many developers prefer to see more of
> the tab title in the tabbar. Lots of "firefox" icons can make it hard to
> find which tab the developer is looking for.

Okay, that makes sense.
Attachment #8956713 - Attachment is obsolete: true
> Nicholas, do the colours (on most people's desktops) have any significance?

For the most part no.

- Ones like .treeline, .mrValue, .mrName, .mrNote are just all slightly different colours because that makes it easier to read than if they're all the same colour.

- .accuracyWarning, .badInputWarning, and .invalid are all some variant of red, because they indicate that something has gone wrong.

I'm not wedded to the specific colours, but if we could preserve those goals (slightly different colours for the first group, and a "warning" indication for the second group) that would be nice.
Flags: needinfo?(n.nethercote)
Attached file colors.html
Well, we could use -moz-activehyperlinktext for the red, and alter the opacity of the element to get various shades of red. And then also use -moz-hyperlinktext if we wanted a separate color (blue), again altering the opacity as needed.
Flags: needinfo?(jaws)
Hi Jaws,

(BTW, it's funny you do accessibility work since your name is the same as a screenreader.  If I'm the dozenth person to point that out I'm sorry, if I'm the first I hope it's amusing.)

I liked your idea about using the link colour and altering the opacity.  I assumed this would be a little to subtle and might create poor contrast with low opacity.  So instead I used filter: hue-rotate to alter the hue.  This seems to work.  I also noticed that I needed to use -moz-nativehyperlinktext since my -moz-hyperlinktext does not contrast well (it is the default dark blue) with my desktop's customized -moz-Field (dark grey).  I can't do the same for -moz-activehyperlinktext so I'm hoping for the best there.

Thanks.
Attachment #8956712 - Attachment is obsolete: true
Attachment #8966950 - Flags: review?(jaws)
BTW. How do you get try to test a change like this?  I just did ./mach build and ./mach run and tested that it worked like I was expecting.
Comment on attachment 8966950 [details] [diff] [review]
Bug 1443396 - Use colours that contrast well on about:memory

Review of attachment 8966950 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay on this. This looks mostly good but upon further thought the changes to use -moz-nativehyperlinktext might work on your machine but might not work well on someone else's setup. I think we should just drop the coloring of the text here, it would be simpler than dealing with the hue-rotate. The color also doesn't appear to infer any information. Some items are clickable and others are not, the only visual way to know is to hover over the item and see the underline appear and the cursor change.

Also, I can't review the /android changes. Snorp, can you review the /android changes?

So, feedback+ on the changes and this can be review+ with the removal of the -moz-nativehyperlinktext.
Attachment #8966950 - Flags: review?(snorp)
Attachment #8966950 - Flags: review?(jaws)
Attachment #8966950 - Flags: feedback+
(In reply to Paul Bone [:pbone] from comment #15)
> (BTW, it's funny you do accessibility work since your name is the same as a
> screenreader.  If I'm the dozenth person to point that out I'm sorry, if I'm
> the first I hope it's amusing.)

Yeah, I get that a lot. It's a coincidence but a fun one :)
Attachment #8966950 - Flags: review?(snorp) → review+
Pushed by pbone@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a78bc7ff3de5
Use colours that contrast well on about:memory r=jaws,snorp
Hi Eliza,

I have no idea how my change could affect that test.  I'm running it in try again now:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66b8085d798b67002c71aaacddf2fd40adc79e5c

I can also wait until bug 1453890 lands before attempting to land this again.

Thanks.
Flags: needinfo?(pbone)
Pushed by pbone@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b20ccf5448b
Use colours that contrast well on about:memory r=jaws,snorp
https://hg.mozilla.org/mozilla-central/rev/9b20ccf5448b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.