24.47 KB, image/gif
17.76 KB, image/gif
51.64 KB, image/gif
7.74 KB, patch
|Details | Diff | Splinter Review|
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.
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
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.
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.
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
Created attachment 19388 [details] As you can see, there's no "Use System Color" option under the colour picker.
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
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
Created attachment 19508 [details] [diff] [review] Change to make a system_colors preference NOT windows only
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: patch, review
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.
matt: Any chance I could get an r= on the C code? Does anyone else think this is the right thing to do? :)
Hi Matt. Were you able to review Mike's patches?
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
coolness! sr=alecf as long as the windows.h issue has been sorted out :)
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
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.
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.
cc'ing pchen, the internet config god
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
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.
whatever. mkaply, you have my sr= if mkaply checks it in, mpt feel free to file a new bug.
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.
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: 18 years ago
Resolution: --- → FIXED
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
* SPAM * Finally got my hands on a new OS/2 build and everything's peachy... well, except for the stock now. :-)
You need to log in before you can comment on or make changes to this bug.