If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Preferences Does Not Allow System Colours To Be Used

VERIFIED FIXED

Status

SeaMonkey
Preferences
P3
normal
VERIFIED FIXED
17 years ago
13 years ago

People

(Reporter: Barry Marshall (at work), Assigned: mkaply)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
OS/2 1114 build, clean install, TP770x, WSeB, SDD, 1024 * 768 * 24bpp

If you load the above URL under NS/2 4.61, the background is white.
Under OS/2 1114 the background is still grey.

I don't recall this being a problem on Win32 1114, but I don't have my 
machine closeby to check.

I'll attach a picture.
(Reporter)

Comment 1

17 years ago
Created attachment 19258 [details]
Mozilla 1114 version partial screen  (note background grey)
(Reporter)

Comment 2

17 years ago
Created attachment 19259 [details]
NS/2 4.61 version partial screen
(Assignee)

Comment 3

17 years ago
This is not necessarily a bug.

This page has not speicified a background color, so the default is used.

The default can be a couple things.

1. A color specified in preferences.

2. If the box in preferences "Use Windows colors" or "Use system colors" is 
checked, it uses a color in the OS. This color is different between 4.61 and 
Mozilla. 4.61 actually had a bug related to this.

Please experiment with your scheme palette and see what results you get. I think 
we use window background color.

Thanks
(Reporter)

Comment 4

17 years ago
Okay, I can confirm #1  (switch background colour yields predicted 
results), but where is #2 to test?

Under NS/2 4.61 I have under Preferences -> Colors an option "Use 
default colors", which I have selected, hence my system white 
background.

Under Warpzilla 1114 I can't find a GUI option to pick system colours.  
There's nothing under "Text and Background" to do this, Color, or any 
other Preferences option I can find.

Comment 5

17 years ago
Okay, I can confirm that Win32 1114 does indeed have a checkbox under the 
default colour selections to enable default system colour values, but OS/2 1114 
doesn't.
(Reporter)

Comment 6

17 years ago
Changed description.  Picture of problem to follow.
Component: Browser-General → Preferences
Summary: OS/2 - White Background Browser Colour Problem → OS/2 - Preferencees Does Not Allow System Colours To Be Used
(Reporter)

Comment 7

17 years ago
Created attachment 19388 [details]
As you can see, there's no "Use System Color" option under the colour picker.
(Assignee)

Comment 8

17 years ago
The code for "Use Windows Colors" should be "Use System Colors" and should be XP.

The preference should be named differently as well, although I 'm not sure how
to do that cleanly and still support the win pref.

I'll attach diffs for this.
Assignee: mkaply → matt
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: OS/2 → All
QA Contact: doronr → sairuh
Summary: OS/2 - Preferencees Does Not Allow System Colours To Be Used → Preferencees Does Not Allow System Colours To Be Used
(Reporter)

Comment 9

17 years ago
SPAM  (Fixed my typo in Summary...  I really must learn to type one of 
these days...   :-)  )
Summary: Preferencees Does Not Allow System Colours To Be Used → Preferences Does Not Allow System Colours To Be Used

Updated

17 years ago
Hardware: PC → All
(Assignee)

Comment 10

17 years ago
Created attachment 19508 [details] [diff] [review]
Change to make a system_colors preference NOT windows only
(Assignee)

Comment 11

17 years ago
OK, I attached a diff for this.

It does the following:

1. Removes Windows specific code in nsPresContext and uses nsLookAndFeel to get 
foreground and background colors. (changed name of pref)

2. Changes some stuff in editor that was using the pref.

3. Changed preferences so the checkbox is in all pref-colors, not just Windows.
keywords
Keywords: patch, review

Comment 13

17 years ago
indentation for pref-colors.dtd looks a bit off.
pref-colors.xul has a section w/ ~120 columns.
-+        <checkbox id="browserUseSystemColors" value="&useSystemColors.label;" 
accesskey="&useSystemColors.accesskey;"
-+                  pref="true" preftype="bool" 
prefstring="browser.display.use_system_colors" prefattribute="checked"/>
++        <checkbox id="browserUseSystemColors"
++          value="&useSystemColors.label;"
++          accesskey="&useSystemColors.accesskey;"
++          pref="true" preftype="bool" prefattribute="checked"
++          prefstring="browser.display.use_system_colors"
++        />
[or some other syntax]
otherwise you could take an r=timeless if you want it.
Keywords: approval
(Assignee)

Comment 14

17 years ago
matt:

Any chance I could get an r= on the C code?

Does anyone else think this is the right thing to do? :)
(Reporter)

Comment 15

17 years ago
Hi Matt.

Were you able to review Mike's patches?

Comment 16

17 years ago
Looks fine to me.  I assume you're sure nsPresContext.cpp no longer needs 
windows.h?  Also, please leave autostretch="never" on the <box/> in pref-
colors.xul.  r=blake, cc'ing alec for sr

Comment 17

17 years ago
coolness!
sr=alecf as long as the windows.h issue has been sorted out :)
(Assignee)

Comment 18

17 years ago
windows.h is definitely only included in nsPresContext.cpp for this color stuff. 
It builds fine without it.

Hurrah. Removing platform specific code from XP :)

Assignee: matt → mkaply

Comment 19

17 years ago
Sorry to be annoying, but take a look at
<http://www.chasms3.com/mac022600/apple7c3.htm>. Is the Mac implementation 
getting the system colors from the Internet control panel? Judging by the call to 
nsILookAndFeel::whatever, it's not, it's just getting it from the Appearance 
Manager theme. That's not right.

Until that's fixed, the GUI for this shouldn't be present on Mac OS, as it won't 
be doing what the user expects.
(Assignee)

Comment 20

17 years ago
I have to humbly disagree.

We can certainly override that pref in mac.js and make it false so that the Mac 
users see no difference in current behavior. But Mac not using Internet Control 
Panel for nsLookAndFeel is a different issue.

I searched various documentation on Internet Config at:

http://www.quinn.echidna.id.au/Quinn/Config/

and found no information on querying the color preferences, or I would implement 
it. Note that there is an InternetConfig API interface already in Mozilla, so 
someone just needs the ability to query those color prefs to it.

Comment 21

17 years ago
cc'ing pchen, the internet config god

Comment 22

17 years ago
I not-so-humbly disagree.  (and agree with mkaply) - the fact that the system
colors are not retrieved from internet config is a seperate issue. 

if anything, we should be updating nsILookAndFeel to support browser-specific
colors, then use that. On platforms which do not have this system color, we
should use the default forground/background color, but reflect that in the
implementation of nsILookAndFeel for that particular platform.

i.e. we need to add new enums to:
http://lxr.mozilla.org/seamonkey/source/widget/public/nsILookAndFeel.h#43

Comment 23

17 years ago
Yes indeed. I'm not saying that the Mac should *never* have this checkbox, just 
that it shouldn't have it *until* the color settings are read from Internet 
Config (rather than from the Appearance Manager). Otherwise we're basically 
lying.

Comment 24

17 years ago
whatever.
mkaply, you have my sr=
if mkaply checks it in, mpt feel free to file a new bug.

Comment 25

17 years ago
Just to jump in and shamelessly repeat what others have said, properly 
retrieving the colors from the right place should indeed be the responsibility 
of nsILookAndFeel; mkaply shouldn't need to do anything except request the 
right metric.  File a new bug on that.
(Assignee)

Comment 26

17 years ago
Bug:

http://bugzilla.mozilla.org/show_bug.cgi?id=5721

Has been open for an eternity for the internet config. Need to add 
nsLookAndFeel/foreground/background/link color to that bug.

Marking this bug fixed.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 27

17 years ago
Will verify on next OS/2 build.  
The current 1/30 build is missing the fixes from 67061.
vrfy fixed on linux and winnt using 2001.02.09.0x comm bits. [mac stuff covered
by bug 5721.]

barry, how does this look for you on os/2? [unless you're still waiting on a
build...]
Status: RESOLVED → VERIFIED
(Reporter)

Comment 29

17 years ago
* SPAM *
Finally got my hands on a new OS/2 build and everything's peachy...  well,
except for the stock now.  :-) 
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.