62678 - View source uses background from prefs and hardcoded text color

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD: Linux build 2000-12-05-08 and Win98 build 2000-12-11-12

STEPS TO REPRODUCE:
1) Set the default background color to black in preferences -> appearance ->
colors.  Set the default foreground color to white.
2)  go to any web page
3)  view the source

ACTUAL RESULTS:
Source is shown black on black

EXPECTED RESULTS:
Source is shown white on black or black on some hardcoded background color.

NOTES:
Since we have an option for syntactic highlighting, it seems that we should not
be pulling the background color from prefs at all here -- we're hardcoding the
text colors, so we should hardcode the background.  If we decide to pull the
background and text color from prefs, we need a prefs panel to configure the
syntax highlighting colors as well.

Comment 1

[sairuh (rarely reading bugmail)]

odd, i cannot repro this. i set text color to white, background to black, as
well as select "always use my chosen colors"; i turn off windows colors pref on
win32. results: the source window has white text on a black background.

winNT and linux  [2000.12.11.12]
mac [2000.12.12.08]

Comment 2

[Boris Zbarsky [:bzbarsky]]

If I choose "always use my chosen colors" then view source is indeed displayed
white on black (and the syntax highlighting disappears).

If I have "use the colors and background specified by the webpage" then the bug
is as described.

Looks like the issue is that the "page" (view source window) is specifying a
font color but not a background color.

Comment 3

[sairuh (rarely reading bugmail)]

ah! making sure the "always use the web page colors" does the trick. vrfy'd on
mac, linux and winnt. thx for the clarification!

cc'ing jce and akkana who're on bug 52154, where view source coloring was
implemented. any thoughts, guys?

Comment 4

[Hixie (not reading bugmail)]

The fix for this should be to switch to using a stylesheet or to switch back to
XML based view source as we used to have. Either way, the fix should involve 
using an external stylesheet and then making the stylesheet groovy. :-)

Comment 5

[Paul Chen]

nav triage team:

Won't stop beta1 for this, but, yes, would be nice to fix this with a 
stylesheet.

Comment 6

[Paul Chen]

Marking nsbeta1- bugs as future to get off the radar

Comment 7

[Boris Zbarsky [:bzbarsky]]

This stylesheet is being created in bug 74486.  I'll fix this once I have that
fixed....

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: patch to set the background to white

Comment 9

[Boris Zbarsky [:bzbarsky]]

How is this?  reviews?

Comment 10

[Marc Attinasi]

White? Why not use system colors, like {background-color: window;} to use the
system's default window color? I guess the user could set that to the same color
that we use for the text too, but then maybe our text color should be
'windowtext' instead of black?

Comment 11

[Boris Zbarsky [:bzbarsky]]

OK, we can certainly do window and windowtext.  I suppose that a user could set
the "window" color to something that's the same as one of our highlighting
colors... That's why I was setting the color to "white" (something we know
should work).

Ultimately, we want some way of letting users configure all these colors if they
want, I guess.  Are there system colors that usefully correspond to what we are
trying to do?  I can't find any...

bug 52154 has a UI spec for a way to select the view source colors.  Do we want
to go ahead and implement something like that?

Comment 12

[timeless]

i'm pretty sure we have a bug to alter the way highlight works, but until then 
let's use window and windowtext.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: patch using window and windowtext

Comment 14

[Marc Attinasi]

Nice Boris! [s]r=attinasi

Comment 15

[timeless]

r=timeless

Comment 16

[timeless]

fix checked in

Comment 17

[rbs]

Resolving as FIXED to get it off the radars.

Comment 18

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

This is still causing problems for some users -- see bug 67448.  The highlight
colors are really designed to be used only with black text on a light grey or
white background -- so why don't you just make that explicit?  One of the golden
rules of specifying colors in CSS is that you should always specify foreground
and background together.  Here you're breaking that by mixing the user's
background color with your specified foreground for the highlight colors.

Reopening (since the summary is still true, only you've fixed the most common
one of the 10 or so foreground colors).

Comment 19

[Boris Zbarsky [:bzbarsky]]

I just tried out some GTK themes.  David is right -- the highlighting still
breaks badly for most of these.

So, attaching another patch.  This does two things:

1)  Use "white" for background and "black" for text
2)  Adds "monospace" as a fallback for the font family.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to not use system colors in viewsource... since we can't do it consistently

Comment 21

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

r=dbaron, although I thought -moz-fixed was basically the same as monospace
except for the font sizing (and inheritance?).

Comment 22

[Boris Zbarsky [:bzbarsky]]

Well... on my setup they seem to look nothing alike.  So....

Comment 23

[Marc Attinasi]

Hey Boris, sorry I sent you off on a bad path with my suggestion to use
windowColor and windowText - I won't make any more suggestions (for this bug)
unless specifically asked :)  sr=attinasi

btw: -moz-fixed and monospace should both be fixed-width, -moz-fixed is special
because it cannot be changed via a pref. and is handled specially in some
internals of layout.

Comment 24

[Blake Ross]

Fix checked in per request.

Comment 25

[sairuh (rarely reading bugmail)]

vrfy fixed: 2001.05.18.13 comm bits - linux, winnt, mac.