The default bug view has changed. See this FAQ.

"Use my colors" preference makes popup/hover frames have transparent background

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: David Harkness, Assigned: Ginn Chen)

Tracking

({access})

Trunk
access
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040803
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040803

Color settings:

* Browser: green-on-black
* Windows XP Appearance: yellow-on-blue

When the "Use my chosen colors" preference is enabled, hover popups
(e.g. email address in Gmail while hovering over the sender's name) and dropdown
lists created using popups (e.g. "Move To..." dropdown on Yahoo Mail) are
created with a *transparent* background -- not the black background color I've
chosen. This makes them barely readable over the content they cover.

Reproducible: Always
Steps to Reproduce:
1. Set background color preference to some non-standard color (e.g. black).
2. Enable "Use my chosen colors" preference on same preference page.
3. View a site with a hover/popup (e.g. popup navigation menus).

Actual Results:  
The popup has a transparent background, making it difficult to read.

Expected Results:  
The popup has the background color selected in Preferences : Appearance :
Colors. No underlying content from the main page should show through.
The "use my colors" preference sets the root to your chosen background and
everything else to transparent.  The other option would be to set backgrounds on
everything in sight and have chunks of text disappear completely (because it's
fairly common for things with transparent background to cover text).

So the basic problem is that the browser has to guess whether something should
have a transparent background or not, and it has nothing to base that guess on.
(Reporter)

Comment 2

13 years ago
Would the following work as a fix and workable solution in general? Instead of
setting the root to my color and everything else to transparant, simply replace
the background color/image with mine for all elements that actually specify one?

Given the wording of the preference, that's what I had assumed was happening and
that there was simply a bug when a hover/popup was involved.

Comment 3

13 years ago
*** Bug 250069 has been marked as a duplicate of this bug. ***

Comment 4

12 years ago
See bug 205892 and bug 247805

Comment 5

12 years ago
This has been in bugzilla for just under two years (205892).

>The "use my colors" preference sets the root to your chosen background 
>and everything else to transparent.

What is the normal behaviour?  The set-everything-else-to-transparent is at best
unneccessary, and seems to be very wrong in most cases.  

David's solution seems to be the way to go (at the very least, it's the
implementation suggested by the description in the user interface)

Comment 6

12 years ago
Make that 2 and a half (160099)

Can anybody point me to the relevant source files?
(Assignee)

Comment 7

12 years ago
*** Bug 205892 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

12 years ago
*** Bug 247805 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 9

12 years ago
*** Bug 160099 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 10

12 years ago
*** Bug 243041 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 11

12 years ago
*** Bug 293740 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 12

12 years ago
(In reply to comment #1)
> So the basic problem is that the browser has to guess whether something should
> have a transparent background or not, and it has nothing to base that guess on.

I don't think the browser has to guess, a reasonable way is to switch any other bgcolor in the document 
to user chosen bgcolor, but keep transparent background in the document untouched.

For example, if user chooses "yellow"as bgcolor and browse a webpage like this,
<DIV bgcolor="white"> Text ...
<DIV bgcolor="transparent"> Popup ...
Mozilla should displays like this,
<DIV bgcolor="yellow"> Text ...
<DIV bgcolor="transparent"> Popup ...
But now with this bug, it's like,
<HTML><BODY bgcolor="yellow"> ...
<DIV bgcolor="transparent"> Text ...
<DIV bgcolor="transparent"> Popup ...
It's no good.

IE doesn't have the same problem.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: access
OS: Windows XP → All
Hardware: PC → All
(Assignee)

Comment 13

12 years ago
cc:bz to let him see the last comment
The point is if we implement this via CSS rules we have to guess.  If we hack
style resolution directly to implement support for this pref, then I agree that
we can do it "right".
(Assignee)

Comment 15

12 years ago
Any plan to do a hack in style resolution or do a better guess?
(Assignee)

Comment 16

12 years ago
(In reply to comment #6)
> Make that 2 and a half (160099)
> 
> Can anybody point me to the relevant source files?

PrefShell::SetPrefColorRules()
> Any plan to do a hack in style resolution or do a better guess?

It seems ok to me if it can be done fast (possibly in SetColor and using a
cached pref on the prescontext?)
Er, ignore what I said about SetColor.  Just using a fast accessor on
prescontext and in the style computation for background color, I guess.
(Assignee)

Comment 19

12 years ago
Created attachment 183896 [details] [diff] [review]
draft patch

a draft patch
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #183896 - Flags: review?(bzbarsky)
(Assignee)

Comment 20

12 years ago
(In reply to comment #19)
> Created an attachment (id=183896) [edit]
> draft patch
> 
> a draft patch
a side-effect:
DOM Inspector can't return correct value of the background-color.
Comment on attachment 183896 [details] [diff] [review]
draft patch

This is wrong; think about background images.

I'm also not clear about why XUL is special here and I'm not too happy with
having the extra work on every single paint.
Attachment #183896 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 22

12 years ago
(In reply to comment #21)
> (From update of attachment 183896 [details] [diff] [review] [edit])
> This is wrong; think about background images.
You're right. I should add "background-image: none !important;".

> I'm also not clear about why XUL is special here and I'm not too happy with
> having the extra work on every single paint.
Because the Fx UI colors should not change due to user chosen "Content->Colors".
There're quite a few UI elements are using hardcoded colors in css files.

Can you suggest another place to do this work?
I think the patch runs fast enough.
> Because the Fx UI colors should not change due to user chosen
> "Content->Colors".

What about HTML files in the UI (like the help, say)?

What about XUL that's served remotely and isn't part of the UI?

Please get out of the "UI is only XUL and XUL is only UI" mindset.

>  Can you suggest another place to do this work?

I already did; please read my comments in this bug (eg comment 14).

> I think the patch runs fast enough.

What performance testing have you done?
(Assignee)

Comment 24

12 years ago
(In reply to comment #23)
> What about HTML files in the UI (like the help, say)?
> What about XUL that's served remotely and isn't part of the UI?
> Please get out of the "UI is only XUL and XUL is only UI" mindset.
I just want to keep the same behavior with XUL and Help before this patch.
I don't want URL box uses user chosen background color.
And the help file is already using user chosen background color before this patch.
Actually  the CSS lines in PrefShell::SetPrefColorRules() only work with HTML, right?
So this hacking should only works with HTML, too.

> I already did; please read my comments in this bug (eg comment 14).
But I can't find a way to seperate HTML and XUL there.

> What performance testing have you done?
I can make a pageload test after my lunch.
> I just want to keep the same behavior with XUL and Help before this patch.

Even if it's wrong?

> Actually the CSS lines in PrefShell::SetPrefColorRules() only work with HTML,
> right?

As written, yes.

> But I can't find a way to seperate HTML and XUL there.

But you could separate content and chrome....

> I can make a pageload test after my lunch.

Actually, dhtml tests would be more useful there.

In any case, the point is that this isn't something we should be doing every
single time we paint.  Style resolution is the right time to do it.
(Assignee)

Comment 26

12 years ago
(In reply to comment #25)
> > Actually the CSS lines in PrefShell::SetPrefColorRules() only work with HTML,
> > right?
> As written, yes.
So the patch's effect need to be narrowed to HTML as same as PrefColorRules.
Otherwise we need an other implementation for both fg color and bg color.

Comment 27

12 years ago
Created attachment 183994 [details] [diff] [review]
draft patch v2

another draft patch
A small concern, nsPresShell is using namespace to narrow the effects,
nsRuleNode is using IsChrome().
There may be some inconsistence with remote XUL.
Anyway, it's not a big matter to me.
Attachment #183994 - Flags: review?(bzbarsky)
(Assignee)

Comment 28

12 years ago
The last comment and the patch is on behalf of me.
I forgot to change the login of this machine. :)
> A small concern, nsPresShell is using namespace to narrow the effects,
> nsRuleNode is using IsChrome().

Actaully, nsPresShell bails out if the docshell is of chrome type and never even
creates the preference stylesheet.  So it looks to me like the narrowing by
namespace for this rule is wrong, as I said....

But anyway, I assume you tested this patch on both chrome and non-chrome, HTML
and not HTML?

Also, please add spaces after the "//" in your comments.
(Assignee)

Comment 30

12 years ago
(In reply to comment #29)
> Actaully, nsPresShell bails out if the docshell is of chrome type and never even
> creates the preference stylesheet.  So it looks to me like the narrowing by
> namespace for this rule is wrong, as I said....
I tried to remove the namespace setting in PrefShell::CreatePreferenceStyleSheet().
The result is OK and even better.
The test case is,
http://www.infodraft.com/~faser/mab/content/mab.xul
The test machine is Linux using GTK2.

But I the namespace is using by SetPrefNoScriptRule() and a lot other rules.
I don't know how to deal with them.

> I tried to remove the namespace setting in 
> PrefShell::CreatePreferenceStyleSheet().

Leave the namespace setting; just replace the "*" in the selector you want not
namespaced with "*|*".
(Assignee)

Comment 32

12 years ago
Created attachment 184066 [details] [diff] [review]
patch v3

Yes, *|* works. Thanks, bz.
Attachment #183896 - Attachment is obsolete: true
Attachment #183994 - Attachment is obsolete: true
Attachment #184066 - Flags: review?(bzbarsky)
(Assignee)

Updated

12 years ago
Attachment #183994 - Flags: review?(bzbarsky)
(Assignee)

Updated

12 years ago
Attachment #184066 - Flags: superreview?(roc)
(Assignee)

Comment 33

12 years ago
bz, do you have further thinking about patch v3?
I haven't had a chance to look at it, and may not for another week or so.
(Assignee)

Updated

12 years ago
Attachment #184066 - Flags: superreview?(roc) → superreview?(bryner)
Comment on attachment 184066 [details] [diff] [review]
patch v3

Looks reasonable to me... David, is this OK with you?  Or will this cause
issues with dynamic changes to that preference?
Attachment #184066 - Flags: superreview?(dbaron)
Attachment #184066 - Flags: superreview?(bryner)
Attachment #184066 - Flags: review?(bzbarsky)
Attachment #184066 - Flags: review+

Comment 36

12 years ago
*** Bug 300240 has been marked as a duplicate of this bug. ***

Comment 37

12 years ago
After applying patch v3, the background color of check boxes and radio buttons
is now the user-selected background color (instead of white), and, since the
selection glyph is always black, the state of these controls is now very
difficult to determine visually.  I think this is a bug.

One possible solution would be to color the selection glyph with the
user-specified foreground color.  Another possible solution would be to ensure
that the background of these controls remains white (the current behavior
without the patch).
(Assignee)

Comment 38

12 years ago
(In reply to comment #37)
> One possible solution would be to color the selection glyph with the
> user-specified foreground color.  Another possible solution would be to ensure
> that the background of these controls remains white (the current behavior
> without the patch).

I think the best solution is to implement native checkbox/radio button support, as we did on Windows.
To color the selection glyph with the user-specified foreground color is acceptable and easier.
Is the issue the question of which of two UA-!important rules wins in the
cascade?  If so, shouldn't the order be fixed?

Comment 40

12 years ago
When will the patch be merged? =]
Comment on attachment 184066 [details] [diff] [review]
patch v3

I suppose we can try this.  The dynamic changes should work given the pref
stylesheet rules will be changing; that's not permanent if we get rid of that
bit of the pref stylesheet entirely, though.  In any case, this will fix some
things and break others, which is why I've been hesitant about it, but I
realized today that the ones it will break are ones that are (and probably
should be) already broken for event handling (bug 102695), so it's a relatively
small set.
Attachment #184066 - Flags: superreview?(dbaron) → superreview+
(Assignee)

Comment 42

12 years ago
Checking in layout/base/nsPresShell.cpp;
/cvsroot/mozilla/layout/base/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.873; previous revision: 3.872
done
Checking in layout/style/nsRuleNode.cpp;
/cvsroot/mozilla/layout/style/nsRuleNode.cpp,v  <--  nsRuleNode.cpp
new revision: 1.162; previous revision: 1.161
done

David Hilvert, if you can reproduce the issue you mentioned in comment #37,
please file a new bug and attach a testcase, I'm very glad to fix it.
But I can't reproduce it with seamonkey trunk now.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 43

11 years ago
what is the state of this bug? ... in which version of firefox can we expect it to appear?

Comment 44

11 years ago
W.r.t comment #43, status says 'resolved fixed', and the transparency problem is indeed fixed in current versions of Firefox.  I'm still (see comment #37) seeing black (as opposed to custom-color) glyphs against custom backgrounds in checkboxes and radio buttons in the browser version I'm using (Firefox 1.6a1 alpha 2), making it very difficult to distinguish the glyph when the custom background is dark, but I'm too lazy at the moment to check whether this occurs in whatever version is most current.  Perhaps someone else could do this.

Updated

10 years ago
Duplicate of this bug: 368473
Duplicate of this bug: 348637
The issues that this patch caused should be fixed in the patch I'm going to attach to bug 58048.  (I was going to add it to bug 348637, but then I noticed it was already fixed.)
Additional problems fixed by checkin of bug 58048.
Flags: in-testsuite?

Updated

10 years ago
Duplicate of this bug: 401238

Comment 50

9 years ago
Please reactivate this bug. Although resolved fixed in 2005, the current version of SeaMonkey 1.1.12, on Win32, Linux x86 and x86_64, is still having this problem. All DHTML menus on the web, such as the "more" dropdown on the Google main page, still show up without a background if "only my colors" option is checked. Since the menu text is thus superimposed over the underlying page text, the menus are totally unusable in some circumstances.
Mike, this bug wasn't fixed until September 23 2005.  The version of Seamonkey you're using uses a Gecko from early August 2005, with security fixes added.

Once there is a shipping Seamonkey based on Gecko 1.9 you will get this fix in Seamonkey.
You need to log in before you can comment on or make changes to this bug.