Open Bug 35146 Opened 24 years ago Updated 7 months ago

Underlines retain color while being selected

Categories

(Core :: DOM: Selection, defect, P5)

Product:
Core
Component:
DOM: Selection
Platform:
x86
Windows 98
Type:
defect

Tracking

()

People

(Reporter: jruderman, Unassigned)

References

(
URL
)

Details

Attachments

(1 obsolete file) 

Select all the text in the paragraphs at the top of 
<URL:resource:/res/samples/test0.html> and watch the underlines, overlines, and 
strikethroughs.  They retain their color while the text becomes white and the 
background becomes black.  I assume that the "correct" behavior is to turn all 
the lines white, but I'm not sure.

Bug 3836 is another selection bug that happens on the same page.
i agree that the lines should change to the same color as the text.  Every time 
I select text in mozilla i find it awkward that the underlines of links retain 
their color, as this just isn't standard behavior.
that IS funny. I actually went to some trouble to keep all other "decorations" 
the same. whats wrong with this behavior? should the other decorations just not 
show up? I think I will start a discussion of this on the layout newsgroup.
Status: NEW → ASSIGNED
Whatever decision you make:
1) Text with the SMALL-CAPS attribute should be painted using the OS color when 
selected. In the current code, it keeps its foreground color (open test0 and 
select the sentence "THIS TEXT IS SMALL-CAPS").
2) Always check for NS_DONT_CHANGE_COLOR after calling 
GetColor(eColor_TextSelectForeground). On the Mac, we want to keep the foreground 
color for the text and the decorations.

moving to M17 for now
Target Milestone: --- → M17
pierre or kathy. does
2) Always check for NS_DONT_CHANGE_COLOR after calling 
GetColor(eColor_TextSelectForeground). On the Mac, we want to keep the 
foreground 
color for the text and the decorations.

Mean we are or are not doing this right now?
Keywords: nsbeta3, polish
Target Milestone: M17 → M19
Keywords: nsbeta3
moving to future
Keywords: polish → helpwanted
Target Milestone: M19 → Future
*SPAM*: Changing the QA contact of all open/resolved Selection bugs from 
elig@netscape.com to BlakeR1234@aol.com.  After the many great years of service 
Eli has given to Mozilla, it's time for him to move on; he has accepted a 
position at Eazel.  We'll be sad to see him go, and I'll do my best to fill his 
spot...
QA Contact: elig → BlakeR1234
*** Bug 68560 has been marked as a duplicate of this bug. ***
changing selection qa to tpreston.
QA Contact: blaker → tpreston
Attached patch Patch (obsolete) — — Splinter Review 
Here's a patch, that fixes this bug and also incorporates the previous patch I
had in my tree for (very minor) bug 160143.

What we do is that when we, in the PaintTextDecorations() function, come to the
point where we draw the decorations on selections, we check if the selection is
"normal" (i.e., a real selection, no spellchecking underline or something like
that), and then we paint the decorations on the selected characters.

So basically, we first paint all the decorations for a string as usual, and
then if something's selected, we do it again using the correct foreground
selection color.

I've tested this with a number of different testcases; all work well!
Basically the first big chunk of added lines in the patch is what fixes this 
bug. The rest of the changes are for bug 160143.
Comment on attachment 93370 [details] [diff] [review]
Patch

simple fix looks good. I only looked at the fix for bug 35146.
Attachment #93370 - Flags: review+
Reassigning to myself.
Assignee: mjudge → hwaara
Status: ASSIGNED → NEW
Keywords: helpwanted
Target Milestone: Future → ---
How does this interact with the fix for bug 1777?
He will have to merge with my fix, but that is probably simple (for him)...
"probably simple", eh?  There is no way I can see to use the approach this patch
is using once we're painting text-decoration the right way.  For example:

<span style="text-decoration: underline">Text <sup>foo</sup></span>

With the approach in this patch, highlighting the "foo" would actually draw
_two_separate_ underlines if we do text-decorations correctly.  I don't think
that's what we want, is it?
Ok, I'm just saying that it's probably better if Esben (who knows a lot more
about this code, and especially about the new architecture) integrates my fix
into his mega-fix once mine is on the trunk... I don't see what's so
unreasonable about that?
There is no way to "integrate" this fix into the new setup.  It would have to
just be completely redone, using a different approach (and backed out in the
process of landing the new stuff to boot).  So I feel that landing this is not
worth it if the other patch is at all close to landing...
[A review for] this depends on bzbarsky's review for bug 1777, to see if my 
code is bogus with Esben's arch change, or if we can use it in quirks mode at 
least.
Depends on: 1777
*** Bug 118461 has been marked as a duplicate of this bug. ***
Comment on attachment 93370 [details] [diff] [review]
Patch

sr=bzbarsky
Attachment #93370 - Flags: superreview+
*** Bug 164458 has been marked as a duplicate of this bug. ***
*** Bug 180273 has been marked as a duplicate of this bug. ***
Requesting blocking 1.4.We have a patch, (which has both r and sr), so let get
this checked in for 1.4
Flags: blocking1.4?
That patch does not even apply anymore.  It needed to get updated after bug 1777
was fixed, and was not.  At this point it needs updating to trunk and a review
of said updating.

Removing useless blocking request.
Flags: blocking1.4?
Attachment #93370 - Attachment is obsolete: true
I wonder, if this would be fixed, would it not contradict css3 selectors?
http://www.w3.org/TR/css3-selectors/#selection
Afaik, selection is considered there as an anonymous inline box:
<span>The word <span::selection>select</span::selection> is selected</span> 
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050205
Firefox/1.0+

(Aqua Mac OS X 10.2.6)

Selection means a blue background, with the foreground staying the same, and
the recipe in Comment 0 WFM.

FWIW, I think that comment 26 applies, but I don't claims more than a passing
familiarity with CSS3
*** Bug 304832 has been marked as a duplicate of this bug. ***
Assignee: hwaara → selection
QA Contact: tpreston
*** Bug 333053 has been marked as a duplicate of this bug. ***
Assignee: selection → nobody
QA Contact: selection

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.

Severity: minor → S4
Priority: P3 → P5

I am pretty sure this bug is affecting Firefox currently, specifically at least 117.0. This is also an issue in Firefox for Android, at least 117.0.1. If I go to my site (https://tristan.partin.io/) in Firefox and highlight some text containing a link, the underline (text decoration) retains its original non-highlighted color. If I do the same in Chromium, the underline (text decoration) is black as expected.

Here is a minimal reproducer:

<head>
	<style>
		.consistent-link,
		.consistent-link:link,
		.consistent-link:visited {
			color: #7db3e3;
		}

		::selection {
			background-color: #f48024;
			color: black;
		}
	</style>
</head>
<body>
	Hello <a class="consistent-link" href="https://duck.com">World</a>
</body>

If you would like me to attach an image, let me know.

Hmm... I guess I missed that this bug is open, not closed. My apologies :).

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: