Underlines retain color while being selected

RESOLVED INACTIVE

Status

()

Core
Selection
P3
minor
RESOLVED INACTIVE
18 years ago
2 days ago

People

(Reporter: Jesse Ruderman, Unassigned)

Tracking

Trunk
x86
Windows 98
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 obsolete attachment)

(Reporter)

Description

18 years ago
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.

Comment 1

18 years ago
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.

Comment 2

18 years ago
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

Comment 3

18 years ago
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.

Comment 4

18 years ago
moving to M17 for now
Target Milestone: --- → M17

Comment 5

18 years ago
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

Updated

18 years ago
Keywords: nsbeta3

Comment 6

18 years ago
moving to future
Keywords: polish → helpwanted
Target Milestone: M19 → Future

Comment 7

18 years ago
*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

Comment 8

18 years ago
*** Bug 68560 has been marked as a duplicate of this bug. ***

Comment 9

17 years ago
changing selection qa to tpreston.
QA Contact: blaker → tpreston

Comment 10

16 years ago
Created attachment 93370 [details] [diff] [review]
Patch

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!

Comment 11

16 years ago
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 12

16 years ago
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+

Comment 13

16 years ago
Reassigning to myself.
Assignee: mjudge → hwaara
Status: ASSIGNED → NEW
Keywords: helpwanted
Target Milestone: Future → ---
How does this interact with the fix for bug 1777?

Comment 15

16 years ago
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?

Comment 17

16 years ago
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...

Comment 19

16 years ago
[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. ***
*** Bug 164458 has been marked as a duplicate of this bug. ***

Comment 23

16 years ago
*** 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?
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> 

Comment 27

14 years ago
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

Comment 30

2 days ago
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Last Resolved: 2 days ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.