User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040817 Firefox/0.9.1+ Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040817 Firefox/0.9.1+ The Find Toolbar's highlight feature uses yellow highlighting, something which is hardcoded. This is a problem on two occasions: 1) When the background is yellow, or a colour that is near to yellow. http://www.cusser.net/misc/firefox/highlight/testcase.php 2) When the text is yellow, or a colour near to yellow. http://www.cusser.net/misc/firefox/highlight/testcase2.php In the first case, the highlighted text is hardly distinguishable or completely indistinguishable from regular text... In the second case, the text is hardly visible or totally invisible. In my Context Highlight extension (http://extensions.cusser.net/contexthighlight/contexthighlight.xpi) I checked through DOM traversal to find the true background colour behind the text, and chose an appropriate background colour to contrast with it, using the following code: if (colorString.substring(0,3) == 'rgb') { colorString = colorString.substring(4,(colorString.length-1)); colorString = colorString.replace(" ",""); var sArray = colorString.split(","); if ((sArray[0] == 255) && (sArray[1] >= 200) && (sArray[1] <= 255)) contrastRequired++; if ((sArray[1] == 255) && (sArray[0] >= 000) && (sArray[0] <= 255)) contrastRequired++; } While there are prettier and more comprehensive ways of doing this... I believe that we should discuss and implement something similar, since this is a pretty major oversight in the highlight algorithm. Marking major due to the fact that this renders Firefox's highlighter nearly useless on some pages. Reproducible: Always Steps to Reproduce:

Possibly we need to do something like the recent fix for bug 249680 to fix light text over a yellow background on the URL bar. This would not fix the state where the highlight color is close to the page background color. I was also thinking about putting the highlight color into a user-pref since it would be nice if the end-user could change the color to help those who have trouble seeing yellow.

This might be a dup of bug 56314.

I wouldn't have thought so, since bug 56314 is about the colour of mouse selections. This bug is about the highlight colour (yellow) used by the Find Toolbar.

This patch doesn't fix the highlight color contrasting problem, but it will does fix the problem of colored text being seen on the yellow highlight. I have to think some more about (and figure out how to..) fix the problem of the highlight color contrasting problem. The color or all text highlighted is changed to black (like the recent change for secure URL's) to ensure visibility of the highlighted string.

*** Bug 267429 has been marked as a duplicate of this bug. ***

*** Bug 271653 has been marked as a duplicate of this bug. ***

Same change as the last patch, just updated for new firefox 1.0 findBar.js file

Small formatting change for a one-line patch

Updated patch with suggestions from Gavin to put the hard-coded text color in the same location as the hard-coded background color. This makes future changes to colors/possible additions of preferences easier.

this is okay, IF we don't care about yellow backgrounds. However, if this is going to be a "just good enough to make the real fix unnecessary" situation, maybe we shouldn't go this way on things. Also, isn't the find highlighting hardcoded as well? Flipside to any dynamic highlighting solution is that we may run into problems if the highlight/find colours aren't consistent enough. This requires more thought than 5 AM allows for, so I'll hold off on r+ the patch looks fine though.

(In reply to comment #10) > this is okay, IF we don't care about yellow backgrounds. When I started looking into this, I could find no way to get the background (RGB) color of selected text. I was thinking about adding 4 preferences for: Highlight Backgroung Color 1 Highlight Text Color 1 Highlight Backgroung Color 2 Highlight Text Color 2 Then setting background color 1 to yellow, and background color 2 to a light-blue (Both texts black). Between yellow and blue we can visibly highlight any background color. The reason for the preferences was to allow people with color-sensitivity problems to change them to a color more suited to them. At the time there was some backlash about there being to many preferences, and the overhead cost of loading them getting high. Since I could find no way to discover the background color of selected text, there really was no reason to add them. The current patch does not solve the background color situation, but personally in my browsing I find the text color issue a much more promenent problem (light text on dark backgrounds). With Gavin's suggestion of putting the hard-coded colors in the same area, any future dynamic color changes would still probably be able to use the code in this patch (as long as we stay with .js)

It's easy to get the background colour of text nodes, I employ a few short functions in my Context Highlight extension (version 0.2) to establish the RGB values. It's probably more comprehensive than anything I'd expect to be put into Firefox by default, but if you're working on a patch for this bug, it's probably worth checking out... You can find it at http://www.cusser.net/extensions/contexthighlight/

I have a following idea. 1. set text color. (e.g., color=black, bgcolor=yellow) 2. add new css property that name is "-moz-auto-color-reverse". 3. change highlight element to "<span id="__firefox-findbar-search-id" style="color: black; background-color: yellow; -moz-auto-color-reverse: auto;">". 4. in nsTextFrame.cpp, if -moz-auto-color-reverse is auto, check that the background color and text color has suficient contrast by same algorithm as bug 56314.

Comment on attachment 169793 [details] [diff] [review] make highlighted text black on yellow sorry this took so long. I could swear I reviewed this already.

Comment on attachment 169793 [details] [diff] [review] make highlighted text black on yellow Do it need superreview?

(In reply to comment #15) > Do it need superreview? No superreview is needed for toolkit, see http://www.mozilla.org/projects/toolkit/review.html .

Comment on attachment 169793 [details] [diff] [review] make highlighted text black on yellow a=shaver

checked-in. But we have a problem that is background-color contrast problem.

This is fixed by bug 263683.