Open Bug 35146 Opened 26 years ago Updated 2 years 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: