289652 - [regression] -moz-selection fails to work

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[philippe (part-time)]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050408 Firefox/1.0+ (PowerBook)
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050408 Firefox/1.0+ (PowerBook)

On nightly trunk builds (Firefox 1.0+), since 20050405, the browser fails to
render anything with -moz-selection. I'll attach a test case.

Safari 1.2 OS X Panther renders the test correctly.

possible result from bug 56314 ?

Reproducible: Always

Steps to Reproduce:
1.add ::-moz-selection selector to stylesheet
2.select text in a page
3.

Actual Results:  
the background for the selected text takes the default background for selection
as dictated by the OS

Expected Results:  
the background for the selected text should take the background as specified by
the selector.

Comment 1

[philippe (part-time)]

Attachment: test case

I've added the official (css3) ::selection as well, which is supported by
Safari 1.2

Comment 2

[Hiro]

(In reply to comment #0)
> possible result from bug 56314 ?
Yes.
-moz-selection has been decided not to be supported by discussing bug 56314 in Mac. 
It might be INVALID. 

Comment 3

[philippe (part-time)]

(In reply to comment #2)
...
> -moz-selection has been decided not to be supported by discussing bug 56314 in
Mac. 
> It might be INVALID.


Safari (1.2), OS X 10.3 does support ::selection. 
I completely fail to see the reasons why Gecko should not support this on OS X.

The problem in bug 56314 is related to a computation to automatically change the
background-colour of selected test to improve contrast. Which is not a bad idea
by the way. I can possibly understand why this shouldn't be implemented for OS X
builds. Safari doesn't do that either, at the moment.

But from there it is a big step to completely disable ::-moz-selection, which
has additional benefits for both the stylesheet author and the user, for example
in terms of accessibility.

Comment 4

[Martijn Wargers (dead)]

when I remove the first selection p[class="test"]::selection, everything works
all right. 
That part Mozilla doesn't understand, that's why the whole rule is thrown out. 
As a consequence it isn't working in Mozilla1.7 also.
So I think this bug can be resolved WFM.

Comment 5

[philippe (part-time)]

Attachment: test case 2

Same testcase as the first one, but deleted the ::selection selector.

This does *not* work with Firefox, OS X. (but works with Windows and Linux, as
far as I know)

Comment 6

[Martijn Wargers (dead)]

Ah, ok. Marking new, then.
So testcase2 works with a 20050404 nightly MacOSX build, but fails with a
20050405 build, right?

Comment 7

[philippe (part-time)]

(In reply to comment #6)
>...
> So testcase2 works with a 20050404 nightly MacOSX build, but fails with a
> 20050405 build, right?

Actually, it is slightly older. It works with 20050403 OS X, but not with
20050404. I'll attach a screenshot in a moment.

Comment 8

[philippe (part-time)]

Attachment: screenshot

Screenshot from 20050403 Official build, OS X (10.3.8), working
::-moz-selection.

Comment 9

[Martijn Wargers (dead)]

Well in that regression timeframe:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-04-03+04%3A00%3A00&maxdate=2005-04-04+12%3A00%3A00&cvsroot=%2Fcvsroot
The most likely candidate for this regression (which I can't see, since I don't
have a Mac) is bug 56314.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I removed ::-moz-selection support on Mac in bug 56314.

Comment 11

[philippe (part-time)]

(In reply to comment #10)
> I removed ::-moz-selection support on Mac in bug 56314.

That is why I filed this bug.
See my idea about this in comment #3 above.

And I point out again that Safari 1.2 does support ::selection as per CSS3
selectors. I'll attach a screenshot from Safari.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

See bug 56314 comment 57.

Comment 13

[philippe (part-time)]

Attachment: Screenshot from Safari

Screenshot with Safari 1.2 from the same testcase, but using the ::selection
pseudo-element instead. (http://www.w3.org/TR/css3-selectors/#UIfragments).

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that if we support ::-moz-selection on Mac, it should not support
background only, we should support both background and foreground.
If not so, Mac users cannot see the selection text with following style.

p{ color: #002200; background-color: white; }
p::-moz-selection{ color: white; background-color: black; }

Comment 15

[philippe (part-time)]

Color should certainly be supported.
Ideally, Firefox should support the four properties mentioned in CSS3 selectors:
color, cursor, background, outline. Safari currently supports color and
background(-color)

Comment 16

[Boris Zbarsky [:bzbarsky]]

So to make it clear what's going on here:

1)  Before the patch in bug 56314, ::-moz-selection could change the background
    color but not the foreground color, on Mac.
2)  That patch made the behavior consistent.
3)  We probably should allow styling both, even on Mac.
4)  ::-moz-selection is not quite the same as CSS3 ::selection (that's why it's
    named differently).   It's worth keeping that in mind.

Masayuki Nakano, do you feel ok with changing this code per item 3?

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yes.
I like that it is same behavior on all platforms.

i.e.,
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsTextFrame.cpp#1238
- 1238   if (!dontChangeTextColor &&
- 1239       mSelectionPseudoStyle &&
+        if (mSelectionPseudoStyle &&
  1240       mSelectionStatus == nsISelectionController::SELECTION_ON) {

Comment 18

[Boris Zbarsky [:bzbarsky]]

Looks good if the comment right before that is removed and the declaration of
dontChangeTextColor moved below this block.

Comment 19

[Boris Zbarsky [:bzbarsky]]

we want to fix this for 1.8.

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.1

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 183720 [details] [diff] [review]
Patch rv1.1

Requesting 1.8b2 approval for reenabling -moz-selection support on Mac.

Comment 23

[Asa Dotzler [:asa]]

Comment on attachment 183720 [details] [diff] [review]
Patch rv1.1

a=asa

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in.